Re: [PATCH] fts, getcwd, glob: audit for dirfd returning -1

[Jim Meyering]

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.