Paul Eggert wrote:
> I audited fts, getcwd, and glob for the possibility of dirfd returning
> -1, and propose the following patch to fix all the problems I saw.
>
> * For fts, rewrite to avoid dirfd entirely. This is doable now that
> the fdopendir replacement doesn't close its argument. The trick is
> to never invoke opendir.
>
> * For getcwd, do not call dirfd. A call is no longer needed now that
> the fdopendir replacement doesn't close its argument.
>
> * For glob, add a comment explaining why dirfd can't return -1 there.
...
> diff --git a/lib/fts.c b/lib/fts.c
...
> -opendirat (int fd, char const *dir, int extra_flags)
> +opendirat (int fd, char const *dir, int extra_flags, int *pdir_fd)
> {
> int new_fd = openat (fd, dir,
> (O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
> @@ -307,6 +305,7 @@ opendirat (int fd, char const *dir, int extra_flags)
> close (new_fd);
> errno = saved_errno;
> }
> + *pdir_fd = new_fd;
> return dirp;
> }
Nice patches. Thanks!
I would make one change: in opendirat, set *pdir_fd only
when the function returns non-NULL:
diff --git a/lib/fts.c b/lib/fts.c
index 40f1dee..de01244 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -305,7 +305,10 @@ opendirat (int fd, char const *dir, int extra_flags, int
*pdir_fd)
close (new_fd);
errno = saved_errno;
}
- *pdir_fd = new_fd;
+ else
+ {
+ *pdir_fd = new_fd;
+ }
return dirp;
}
I don't particularly like the braces around the single-stmt
block, but since it's an else, IMHO, they're required.
Reverse the condition and put the single-stmt block first
(with no braces) if you'd prefer.
