[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Findutils-patches] [PATCH] find: handle more readdir(3) errors
From: |
Eric Blake |
Subject: |
Re: [Findutils-patches] [PATCH] find: handle more readdir(3) errors |
Date: |
Tue, 1 Nov 2016 15:06:20 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
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