michallenc commented on code in PR #18697:
URL: https://github.com/apache/nuttx/pull/18697#discussion_r3068271275
##########
fs/fat/fs_fat32dirent.c:
##########
@@ -1121,17 +1138,35 @@ static int fat_path2dirname(FAR const char **path,
/* Assume no long file name */
dirinfo->fd_lfname[0] = '\0';
+ dirinfo->fd_name[0] = '\0';
- /* Then parse the (assumed) 8+3 short file name */
+ /* Parse long file name */
- ret = fat_parsesfname(path, dirinfo, terminator);
+ ret = fat_parselfname(path, dirinfo, terminator);
if (ret < 0)
{
- /* No, the name is not a valid short 8+3 file name. Try parsing
- * the long file name.
+ return ret;
+ }
+ else
+ {
+ /* Does the long file name fit into short file name? This means
+ * this is actually a short file name.
+ *
+ * There are following possibilities:
+ * - file was created on Linux and is written as long file name
+ * - file was created on Windows and is written as short file name
+ * - we are creating file on NuttX -> write it as short file name
*/
- ret = fat_parselfname(path, dirinfo, terminator);
+ if (strlen((FAR const char *)dirinfo->fd_lfname) <= DIR_MAXFNAME)
+ {
+ /* Get short file name for given path */
+
+ char *name = kmm_zalloc(DIR_MAXFNAME);
Review Comment:
Because this code (which is basically a simplification of what we have in FAT
```c
static int fnc(const char **path) {
const char *node = *path;
int ch = *node++;
return 0;
}
int main() {
const char *p = "file";
char path[11];
memcpy(path, p, 11);
fnc((const char **)&path);
return 0;
}
```
leads to this:
```
michal@desktop:~/Michal/codes/c$ gcc test.c && valgrind ./a.out
==9122== Memcheck, a memory error detector
==9122== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==9122== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==9122== Command: ./a.out
==9122==
==9122== Invalid read of size 1
==9122== at 0x109148: fnc (in /home/michal/Michal/codes/c/a.out)
==9122== by 0x109187: main (in /home/michal/Michal/codes/c/a.out)
==9122== Address 0x656c6966 is not stack'd, malloc'd or (recently) free'd
==9122==
==9122==
==9122== Process terminating with default action of signal 11 (SIGSEGV)
==9122== Access not within mapped region at address 0x656C6966
==9122== at 0x109148: fnc (in /home/michal/Michal/codes/c/a.out)
==9122== by 0x109187: main (in /home/michal/Michal/codes/c/a.out)
==9122== If you believe this happened as a result of a stack
==9122== overflow in your program's main thread (unlikely but
==9122== possible), you can try to increase the size of the
==9122== main thread stack using the --main-stacksize= flag.
==9122== The main thread stack size used in this run was 8388608.
==9122==
==9122== HEAP SUMMARY:
==9122== in use at exit: 0 bytes in 0 blocks
==9122== total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==9122==
==9122== All heap blocks were freed -- no leaks are possible
==9122==
==9122== For lists of detected and suppressed errors, rerun with: -s
==9122== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault
```
And we can't change `fat_parsesfname` to expect the size will be always only
up to 11 characters, because it won't - it is used to parse the entire file
path with `/` character as a terminator. We could have a different
implementation of `fat_parsesfname`, but I think it's better to `malloc` here
instead of unnecessary code duplication.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
