diff mbox series

xfs: Zero initialize highstale and lowstale in xfs_dir2_leafn_add

Message ID 20190308003413.25068-1-natechancellor@gmail.com (mailing list archive)
State Accepted
Headers show
Series xfs: Zero initialize highstale and lowstale in xfs_dir2_leafn_add | expand

Commit Message

Nathan Chancellor March 8, 2019, 12:34 a.m. UTC
When building with -Wsometimes-uninitialized, Clang warns:

fs/xfs/libxfs/xfs_dir2_node.c:481:6: warning: variable 'lowstale' is
used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
fs/xfs/libxfs/xfs_dir2_node.c:481:6: warning: variable 'highstale' is
used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]

While it isn't technically wrong, it isn't a problem in practice because
highstale and lowstale are only initialized in xfs_dir2_leafn_add when
compact is not zero then they are passed to xfs_dir3_leaf_find_entry,
where they are initialized before use when compact is zero. Regardless,
it's better not to be passing around uninitialized stack memory so zero
initialize these variables, which silences this warning.

Link: https://github.com/ClangBuiltLinux/linux/issues/393
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 fs/xfs/libxfs/xfs_dir2_node.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Darrick J. Wong March 8, 2019, 12:47 a.m. UTC | #1
On Thu, Mar 07, 2019 at 05:34:13PM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
> 
> fs/xfs/libxfs/xfs_dir2_node.c:481:6: warning: variable 'lowstale' is
> used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
> fs/xfs/libxfs/xfs_dir2_node.c:481:6: warning: variable 'highstale' is
> used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
> 
> While it isn't technically wrong, it isn't a problem in practice because
> highstale and lowstale are only initialized in xfs_dir2_leafn_add when
> compact is not zero then they are passed to xfs_dir3_leaf_find_entry,
> where they are initialized before use when compact is zero. Regardless,
> it's better not to be passing around uninitialized stack memory so zero
> initialize these variables, which silences this warning.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/393
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_dir2_node.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 3b03703c5c3d..de46f26c5292 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -444,6 +444,8 @@ xfs_dir2_leafn_add(
>  
>  	dp = args->dp;
>  	leaf = bp->b_addr;
> +	highstale = 0;
> +	lowstale = 0;

I think it would be a good idea to clean all the typedef junk out of the
function definition as well... but on its own this will at least shut up
the smatch/clang warnings, so:

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

(I'll have this and the cleanup tested and ready tomorrow, probably...)

Thanks for the contribution!

--D

>  	dp->d_ops->leaf_hdr_from_disk(&leafhdr, leaf);
>  	ents = dp->d_ops->leaf_ents_p(leaf);
>  
> -- 
> 2.21.0
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 3b03703c5c3d..de46f26c5292 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -444,6 +444,8 @@  xfs_dir2_leafn_add(
 
 	dp = args->dp;
 	leaf = bp->b_addr;
+	highstale = 0;
+	lowstale = 0;
 	dp->d_ops->leaf_hdr_from_disk(&leafhdr, leaf);
 	ents = dp->d_ops->leaf_ents_p(leaf);