fts: refactor to use fd-relative operations internally#2278
Conversation
|
Thank you for taking the time to contribute to FreeBSD! There is an issue that needs to be resolved: Note Please review CONTRIBUTING.md, then update and push your branch again. |
Replace all _open(".", O_RDONLY|O_CLOEXEC) calls with
_openat(ROOTFD(sp), ".", O_RDONLY|O_CLOEXEC) in __fts_open(),
fts_read(), and fts_children().
Add ftsp_rootfd to struct _fts_private to hold the root file
descriptor, and FTSP()/ROOTFD() macros for clean access to it.
fts_open() initialises ftsp_rootfd to AT_FDCWD which preserves
existing behaviour.
This is a preparatory change for fts_openat() which will allow
callers to provide a pre-opened directory fd, enabling fts(3)
traversal inside Capsicum capability mode.
Sponsored by: Google LLC (GSoC 2026)
asomers
left a comment
asomers
left a comment
There was a problem hiding this comment.
What about the _open in fts_safe_changedir? I think that instead of changing directories, it would be better to open the child directory, and use that for future openat calls. But since the child directory is already opened, there might not be anything to do. In fact, we might be able to remove fts_safe_changedir altogether.
| /* Allocate/initialize the stream. */ | ||
| if ((priv = calloc(1, sizeof(*priv))) == NULL) | ||
| return (NULL); | ||
| priv->ftsp_rootfd = AT_FDCWD; |
There was a problem hiding this comment.
This is only going to work for small directory trees. For larger ones, fts might internally chdir. Then AT_FDCWD would be pointing to the wrong place. So it would be better to use an actual file descriptor. Like fts_rfd. Why can't you just use that?
| struct statfs ftsp_statfs; | ||
| dev_t ftsp_dev; | ||
| int ftsp_linksreliable; | ||
| int ftsp_rootfd; |
There was a problem hiding this comment.
Why add a new file descriptor, instead of reusing fts_rfd?
| if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { | ||
| if ((p->fts_symfd = _open(".", O_RDONLY | O_CLOEXEC, | ||
| 0)) < 0) { | ||
| if ((p->fts_symfd = _openat(ROOTFD(sp), ".", |
There was a problem hiding this comment.
If we've already changed directories, then ROOTFD(sp) will be the wrong thing to use here.
There was a problem hiding this comment.
I've changed ROOTFD(sp) to AT_FDCWD for the mid-traversal CWD saves in fts_read() and fts_children().
| if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { | ||
| if ((p->fts_symfd = | ||
| _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) { | ||
| _openat(ROOTFD(sp), ".", O_RDONLY | |
There was a problem hiding this comment.
Just as above, ROOTFD(sp) will be wrong if we've already changed directories.
| return (sp->fts_child = fts_build(sp, instr)); | ||
|
|
||
| if ((fd = _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) | ||
| if ((fd = _openat(ROOTFD(sp), ".", O_RDONLY | O_CLOEXEC, 0)) < 0) |
There was a problem hiding this comment.
Here, too, ROOTFD(sp) might be wrong.
if the child directory is already opened via fts_safe_changedir, we might not need path-based opens at all. |
Jeetubhati
force-pushed
the
jbhati/fts-refactor
branch
from
6098548 to
6cf9dd9
Compare
ROOTFD(sp) is only correct for the initial open in __fts_open().
After fts has internally chdir'd, ROOTFD(sp) points to the wrong
directory. Use AT_FDCWD for the mid-traversal CWD saves in
fts_read() and fts_children() which is equivalent to the original
_open('.', ...) behaviour.
Sponsored by: Google LLC (GSoC 2026)
2 participants
Replace all _open(".", O_RDONLY|O_CLOEXEC) calls with _openat(ROOTFD(sp), ".", O_RDONLY|O_CLOEXEC) in __fts_open(), fts_read(), and fts_children().
Add ftsp_rootfd to struct _fts_private to hold the root file descriptor, and FTSP()/ROOTFD() macros for clean access to it. fts_open() initialises ftsp_rootfd to AT_FDCWD which preserves existing behaviour.
This is a preparatory change for fts_openat() which will allow callers to provide a pre-opened directory fd, enabling fts(3) traversal inside Capsicum capability mode.
Sponsored by: Google LLC (GSoC 2026)