Re: [Findutils-patches] [PATCH] find: handle more readdir(3) errors

[Eric Blake]

On 11/01/2016 02:37 PM, Bernhard Voelker wrote:
> Similar to the FTS readdir fix in v4.6.0-72-g155c9d1, handle the last
> two unhandled readdir(3) errors.
> 
> * find/pred.c (pred_empty): Do the above.
> * lib/fdleak.c (get_proc_max_fd): Likewise.  While at it, fix the
> condition to only skip "." and ".."; previously, also other files
> beginning with ".." would have been skipped - that was theoretically,
> as we only expect the FD files in "/proc/self/fd".
> ---
>  find/pred.c  |  8 ++++++++
>  lib/fdleak.c | 21 +++++++++++++++++----
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/find/pred.c b/find/pred.c
> index f7e9b59..93f82b6 100644
> --- a/find/pred.c
> +++ b/find/pred.c

[expanding context]

> @@ -380,6 +380,7 @@ pred_empty (const char *pathname, struct stat *stat_buf, 
> struct predicate *pred_
>         state.exit_status = 1;
>         return false;
>       }
> +      errno = 0;
>        for (dp = readdir (d); dp; dp = readdir (d))

Generally wrong; you have to clear errno before EVERY call to readdir(),
not just the first.  Either with 'for(..;..; errno = 0, dp = readdir
(d))', or by...

>       {
>         if (dp->d_name[0] != '.'
              || (dp->d_name[1] != '\0'
                  && (dp->d_name[1] != '.' || dp->d_name[2] != '\0')))
            {
              empty = false;
>             break;
>           }
>       }

...clearing errno at the end of the loop.  But for _this particular
loop_, the body does not touch errno, which means later iterations are
left with it still 0 from the earlier iterations,

> +      if (errno)
> +     {
> +       /* Handle errors from readdir(3). */
> +       error (0, errno, "%s", safely_quote_err_filename (0, pathname));
> +       state.exit_status = 1;
> +       return false;
> +     }

so I can agree to this part.  However,

> +++ b/lib/fdleak.c

> -      while ((dent=readdir (dir)) != NULL)
> -     {
> +      while (1)
> +        {
> +       errno = 0;
> +       dent = readdir (dir);
> +       if (NULL == dent)
> +         {
> +           if (errno)
> +             {
> +               error (0, errno, "%s", quotearg_n_style (0, 
> locale_quoting_style, path));
> +               good = 0;
> +             }
> +           break;
> +         }
> +
>         if (dent->d_name[0] != '.'
> -           || (dent->d_name[0] != 0
> -               && dent->d_name[1] != 0 && dent->d_name[1] != '.'))
> +           || (dent->d_name[1] != 0
> +               && (dent->d_name[1] != '.' || dent->d_name[2] != 0)))
>           {
>             const int fd = safe_atoi (dent->d_name, literal_quoting_style);

THIS loop is a demonstration of how not to use readdir; safe_atoi() can
clobber errno, so you are no longer guaranteed that each loop iteration
is starting with errno == 0.  You'll have to tweak the patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature