libc: Implement ftw and nftw function #1486
Conversation
|
@yamt and @patacongo since my team membrer also need ftw/nftw function, I provide an implementation, please review it. |
| pathbuf[PATH_MAX] = '\0'; | ||
|
|
||
| return do_nftw(pathbuf, fn, fdlimit, flags, 0); | ||
| } |
There was a problem hiding this comment.
i guess it's supposed to restore the original cwd when finished.
There was a problem hiding this comment.
here(https://pubs.opengroup.org/onlinepubs/009695399/functions/nftw.html) is what the spec said:
FTW_CHDIR
If set, nftw() shall change the current working directory to each directory as it reports files in that directory. If clear, nftw() shall not change the current working directory.
so the spec permit the current work directory change if the caller pass FTW_CHDIR.
There was a problem hiding this comment.
That feature would depend on !defined(CONFIG_DISABLE_ENVIRON),
There was a problem hiding this comment.
other implementation i'm aware of restores the cwd. (netbsd)
glibc seems to do the same.
There was a problem hiding this comment.
Ok, I will save and restore cwd.
| path[base - 1] = '\0'; | ||
| r = chdir(path); | ||
| path[base - 1] = '/'; | ||
| } |
There was a problem hiding this comment.
does this work when the user-given path is relative?
There was a problem hiding this comment.
no, the caller must give the absolute path, otherwise lstat or stat at line 92 will fail first.
There was a problem hiding this comment.
I don't know, because the spec don't see how to handle this? But if ftw/nftw need consider the current work directory, should we modify stat/lstat to support it too? Here is the musl implementation:
https://github.com/bminor/musl/blob/master/src/misc/nftw.c
which don't do any special thing here.
There was a problem hiding this comment.
as the musl impl doesn't seem to do chdir at all, i guess relative path can just work there.
There was a problem hiding this comment.
musl can work, just because stat/lstat/chdir support the cwd concept, here is musl' lstat implementation:
https://github.com/bminor/musl/blob/master/src/stat/lstat.c#L6
you can see lstat use AT_FDCWD flag there.
So the correct fix is enhance the low level file system related functions in nuttx with the cwd support.
libs/libc/dirent/lib_nftw.c
Outdated
|
|
||
| if (dir) | ||
| { | ||
| struct dirent *de; |
There was a problem hiding this comment.
| struct dirent *de; | |
| FAR struct dirent *de; |
libs/libc/dirent/lib_nftw.c
Outdated
| static int | ||
| do_nftw(FAR char *path, nftw_cb_t fn, int fdlimit, int flags, int level) | ||
| { | ||
| DIR *dir = NULL; |
There was a problem hiding this comment.
| DIR *dir = NULL; | |
| FAR DIR *dir = NULL; |
| base, level | ||
| }; | ||
|
|
||
| #ifndef CONFIG_DISABLE_ENVIRON |
There was a problem hiding this comment.
i guess it's more user-friendly to make this compile-time (not define FTW_CHDIR) or runtime error (return EINVAL or something explicitly)
i prefer the former.
see the reference here: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/ftw.h.html Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com> Change-Id: I3b368106a56b0e9d4c653f3ae16304b0a9d55fbc
|
@yamt and @patacongo all issue is addressed, please take a look. |
|
@yamt, please merge this patch if you don't have more concern. |
Summary
see the reference here:
https://pubs.opengroup.org/onlinepubs/009695399/basedefs/ftw.h.html
Impact
New function
Testing