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
