xfs
[Top] [All Lists]

Re: [PATCH] xfs_io: don't leak fd in open -Tr failure case

To: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Subject: Re: [PATCH] xfs_io: don't leak fd in open -Tr failure case
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 27 Feb 2014 14:32:47 -0600
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <530F9F37.8020408@xxxxxxxxxx>
References: <530F9F37.8020408@xxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.3.0
On 2/27/14, 2:25 PM, Eric Sandeen wrote:
> Coverity spotted this.
> 
> It complained that we didn't close the fd before returning in
> this case of incompatible options, but it seems like we should
> just test for the incompatible flags before even trying to open
> the file, no?
> 
> (The open would have failed in any case, but with a somewhat
> cryptic "Invalid argument" - so it's probably better to state
> it plainly and bail immediately.)

So actually, we wouldn't leak, because the open would fail.
So I guess it's not the best subject & description...

-Eric

> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> diff --git a/io/open.c b/io/open.c
> index 6bb0d46..c106fa7 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -342,6 +342,11 @@ open_f(
>       if (optind != argc - 1)
>               return command_usage(&open_cmd);
>  
> +     if ((flags & (IO_READONLY|IO_TMPFILE)) == (IO_READONLY|IO_TMPFILE)) {
> +             fprintf(stderr, _("-T and -r options are incompatible\n"));
> +             return -1;
> +     }
> +
>       fd = openfile(argv[optind], &geometry, flags, mode);
>       if (fd < 0)
>               return 0;
> @@ -349,11 +354,6 @@ open_f(
>       if (!platform_test_xfs_fd(fd))
>               flags |= IO_FOREIGN;
>  
> -     if ((flags & (IO_READONLY|IO_TMPFILE)) == (IO_READONLY|IO_TMPFILE)) {
> -             fprintf(stderr, _("-T and -r options are incompatible\n"));
> -             return -1;
> -     }
> -
>       addfile(argv[optind], fd, &geometry, flags);
>       return 0;
>  }
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>