Open PaulStoffregen opened 3 years ago
I believe the problem is within lfnGetName(), when the filename stored on the FAT volume has a null terminating char.
static size_t lfnGetName(DirLfn_t* ldir, char* name, size_t n) {
uint8_t i;
size_t k = 13*((ldir->order & 0X1F) - 1);
for (i = 0; i < 13; i++) {
uint16_t c = lfnGetChar(ldir, i);
if (c == 0 || k >= (n - 1)) {
k = n - 1;
break;
}
name[k++] = c >= 0X7F ? '?' : c;
}
// Terminate with zero byte.
name[k] = '\0';
return k;
}
Inside the loop, when lfnGetChar() returns zero, k is set to n - 1, which causes the null terminating zero to be written at the end of the user's buffer for the case of short string. The c == 0 case should probably break from the loop without altering k, so the null terminating zero is written at the correct location.
if (c == 0) break; // do not alter k for this case, so the zero termination is written at correct location
if (k >= (n - 1)) {
k = n - 1;
break;
}
Or maybe k should be checked first? I'm not sure if the initial compute of k could be too large, or if that's a non-issue?
I need to rethink how getName works. I suspect there is more than one bug. I think I produced a new bug trying to fix another bug.
Long file names are stored like this in the directory. So I am backing though the directory file.
Nth Long entry with flag LAST_LONG_ENTRY (0x40)
… Additional Long Entries …
1st Long entry 1
Short Entry Associated With Preceding Long Entries
I haven't noticed any other problem.
I verified the bug and the following seems to fix it. I would appreciate if you tested any case you have.
static size_t lfnGetName(DirLfn_t* ldir, char* name, size_t n) {
uint8_t i;
size_t k = 13*((ldir->order & 0X1F) - 1);
for (i = 0; i < 13; i++) {
uint16_t c = lfnGetChar(ldir, i);
if (c == 0 || k >= (n - 1)) {
// k = n - 1; <<-------removed
break;
}
name[k++] = c >= 0X7F ? '?' : c;
}
// Terminate with zero byte.
if (k >= n) { // <<----------added
k = n - 1; // <<--------- added
} // <<---------added
name[k] = '\0';
return k;
}
Yes, confirmed, that works great.
I tested here by reverting to the original code and checking that the problem did indeed return (turns out not all boards & programs reproduce the problem - many cases default to buffers pre-filled with zeros), and then I put this into FatFileLFN.cpp and it definitely does solve the problem.
Thank for the check. I will post a new beta today.
The bss zeroed array caught me with a quick fix for another case. I now have a sketch that makes all filename lengths, fills the buffer with '?' before each call and uses a series of buffer lengths to verify truncation.
Looks like getName() doesn't always properly null terminate the string it writes into the user's buffer.
Here's the result I'm seeing on a simple program to recursively print directories.