xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()
diff mbox series

Message ID 1476699404-5977-1-git-send-email-geert@linux-m68k.org
State New, archived
Headers show
Series
  • xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()
Related show

Commit Message

Geert Uytterhoeven Oct. 17, 2016, 10:16 a.m. UTC
with gcc 4.1.2:

    fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
    fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function

Indeed, if "count" is zero, the function will return an uninitialized
error value.

While "count" is unlikely to be zero, this function is called through
the public iomap API. Hence fix this by preinitializing error to zero.

Fixes: 2a06705cd5954030 ("xfs: create delalloc extents in CoW fork")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 fs/xfs/xfs_reflink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig Oct. 17, 2016, 12:08 p.m. UTC | #1
On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
> with gcc 4.1.2:
> 
>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
> 
> Indeed, if "count" is zero, the function will return an uninitialized
> error value.
> 
> While "count" is unlikely to be zero, this function is called through
> the public iomap API. Hence fix this by preinitializing error to zero.

The iomap API should never call in with a zero count, but I think the
initialization is a fine safety net anyway:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Btw, what compiler did you get this from?  I haven't seen it, but then
again I recently missed a lot of initializers without compiler warnings,
so either something changed in the Debian stable gcc or our build
system recently..
Geert Uytterhoeven Oct. 17, 2016, 12:13 p.m. UTC | #2
Hi Christoph,

On Mon, Oct 17, 2016 at 2:08 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
>> with gcc 4.1.2:
>>
>>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
>>
>> Indeed, if "count" is zero, the function will return an uninitialized
>> error value.
>>
>> While "count" is unlikely to be zero, this function is called through
>> the public iomap API. Hence fix this by preinitializing error to zero.
>
> The iomap API should never call in with a zero count, but I think the
> initialization is a fine safety net anyway:

Exactly my thought.

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

> Btw, what compiler did you get this from?  I haven't seen it, but then
> again I recently missed a lot of initializers without compiler warnings,
> so either something changed in the Debian stable gcc or our build
> system recently..

m68k-linux-gnu-gcc version 4.1.2 20061115 (prerelease) (Ubuntu 4.1.1-21)

Stoneage, but it did find 4 real bugs introduced in v4.9-rc1...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Darrick J. Wong Oct. 17, 2016, 6:36 p.m. UTC | #3
On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
> with gcc 4.1.2:
> 
>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
> 
> Indeed, if "count" is zero, the function will return an uninitialized
> error value.
> 
> While "count" is unlikely to be zero, this function is called through
> the public iomap API. Hence fix this by preinitializing error to zero.
> 
> Fixes: 2a06705cd5954030 ("xfs: create delalloc extents in CoW fork")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_reflink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5965e9455d91e036..d48a7cc2fe007f66 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -324,7 +324,7 @@
>  	struct xfs_mount	*mp = ip->i_mount;
>  	xfs_fileoff_t		offset_fsb, end_fsb;
>  	bool			skipped = false;
> -	int			error;
> +	int			error = 0;
>  
>  	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
>  
> -- 
> 1.9.1
>

Patch
diff mbox series

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 5965e9455d91e036..d48a7cc2fe007f66 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -324,7 +324,7 @@ 
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_fileoff_t		offset_fsb, end_fsb;
 	bool			skipped = false;
-	int			error;
+	int			error = 0;
 
 	trace_xfs_reflink_reserve_cow_range(ip, offset, count);