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...
Cheers
Trond
--
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
