On Wed, 03 Jun 2009 16:03:41 -0400
Trond Myklebust <trond.myklebust@xxxxxxxxxx> wrote:
> On Wed, 2009-06-03 at 15:44 -0400, Jeff Layton wrote:
> > The closeall function is broken in such a way that it almost never
> > closes any file descriptors. It's calling strtol on the text
> > representation of the file descriptor, and then checking to see if the
> > value of *endptr is not '\0' before trying to close the file. This check
> > is wrong.
> >
> > When strtol returns an endptr that points to a NULL byte, that indicates
> > that the conversion was completely successful. I believe this check
> > should instead be requiring that endptr is pointing to '\0' before closing
> > the fd.
> >
> > Also, fix up the function to check for conversion errors from strtol. If
> > one occurs, just skip the close on that entry.
> >
> > Reported-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > support/nfs/closeall.c | 4 +++-
> > 1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/support/nfs/closeall.c b/support/nfs/closeall.c
> > index cc7fb3b..5fe07de 100644
> > --- a/support/nfs/closeall.c
> > +++ b/support/nfs/closeall.c
> > @@ -7,6 +7,7 @@
> > #include <unistd.h>
> > #include <stdlib.h>
> > #include <dirent.h>
> > +#include <errno.h>
> >
> > void
> > closeall(int min)
> > @@ -18,8 +19,9 @@ closeall(int min)
> >
> > while ((d = readdir(dir)) != NULL) {
> > char *endp;
> > + errno = 0;
> > long n = strtol(d->d_name, &endp, 10);
> > - if (*endp != '\0' && n >= min && n != dfd)
> > + if (!errno && *endp == '\0' && n >= min && n != dfd)
> > (void) close(n);
> > }
> > closedir(dir);
>
> Shouldn't you also check the case endp == d->d_name? I don't think that
> actually sets errno...
>
Hmm...for that to be a problem, readdir would have to return a name of
"" (i.e. d->d_name would have to point to a NULL byte). Is that a
possibility here? If not, then the *endp == '\0' check should catch
that case too...
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
