Re: [xfs:for-next 70/70] fs/xfs/xfs_da_btree.c:153:26: sparse: symbol 'xfs_da_node_buf_ops' was not declared. Should it be static?

[Dave Chinner <david@fromorbit.com>]

On Fri, Nov 16, 2012 at 03:25:36PM +0800, kbuild test robot wrote:
> tree:   git://oss.sgi.com/xfs/xfs for-next
> head:   1813dd64057490e7a0678a885c4fe6d02f78bdc1
> commit: 1813dd64057490e7a0678a885c4fe6d02f78bdc1 [70/70] xfs: convert buffer verifiers to an ops structure.
> 
> 
> sparse warnings:
> 
> + fs/xfs/xfs_da_btree.c:153:26: sparse: symbol 'xfs_da_node_buf_ops' was not declared. Should it be static?
> --
> + fs/xfs/xfs_dir2_leaf.c:82:1: sparse: symbol 'xfs_dir2_leafn_read_verify' was not declared. Should it be static?
> + fs/xfs/xfs_dir2_leaf.c:89:1: sparse: symbol 'xfs_dir2_leafn_write_verify' was not declared. Should it be static?
> --
> fs/xfs/xfs_dquot.c:55:15: sparse: symbol 'xfs_dqerror_target' was not declared. Should it be static?
> fs/xfs/xfs_dquot.c:56:5: sparse: symbol 'xfs_do_dqerror' was not declared. Should it be static?
> fs/xfs/xfs_dquot.c:57:5: sparse: symbol 'xfs_dqreq_num' was not declared. Should it be static?
> fs/xfs/xfs_dquot.c:58:5: sparse: symbol 'xfs_dqerror_mod' was not declared. Should it be static?
> + fs/xfs/xfs_dquot.c:294:1: sparse: symbol 'xfs_dquot_buf_write_verify' was not declared. Should it be static?
> 
> Please consider folding the attached diff :-)

No, for the same reason as the last one. Though I'll fix the new
ones (the read/write verifier functions) as they should now be
static as a separate patch.

> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 0e92d12..3216738 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -4180,7 +4180,7 @@ error0:
>  /*
>   * Add bmap trace insert entries for all the contents of the extent records.
>   */
> -void
> +static void
>  xfs_bmap_trace_exlist(
>  	xfs_inode_t	*ip,		/* incore inode pointer */
>  	xfs_extnum_t	cnt,		/* count of entries in the list */

And, again, there are lots of changes in this that are unrelated to
the patch.  In this case, the change is plain wrong. It's a debug
only function, called via the macro XFS_BMAP_TRACE_EXLIST:

$ git grep XFS_BMAP_TRACE_EXLIST
fs/xfs/xfs_bmap.c:      XFS_BMAP_TRACE_EXLIST(ip, i, whichfork);
fs/xfs/xfs_bmap.h:#define       XFS_BMAP_TRACE_EXLIST(ip,c,w)   \
fs/xfs/xfs_bmap.h:#define       XFS_BMAP_TRACE_EXLIST(ip,c,w)
fs/xfs/xfs_inode.c:             XFS_BMAP_TRACE_EXLIST(ip, nex, whichfork);
fs/xfs/xfs_inode.c:     XFS_BMAP_TRACE_EXLIST(ip, nrecs, whichfork);

And so it clearly needs to be non-static.

If you are going throw commit-by-commit build warnings and patches
to fix them, please only include the fixes for the *new* warnings
generated by a single commit, not an aggregate of everything that is
found.  For that reason, I think I'd prefer it if your build bot
just sent build warnings, not patches.

FWIW, what happens when a problem is fixed by a later patch in the
tree in the same push? Do you still throw a mail out to the list?
i.e. are you culling spurious warning detections?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs