Re: [PATCH] xfs: Zero initialize highstale and lowstale in xfs_dir2_leafn_add

From: Darrick J. Wong
Date: Thu Mar 07 2019 - 19:48:19 EST


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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>

(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
>