xfs
[Top] [All Lists]

Re: [xfs:for-next 70/70] fs/xfs/xfs_da_btree.c:153:26: sparse: symbol 'x

To: kbuild test robot <fengguang.wu@xxxxxxxxx>
Subject: 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?
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sun, 18 Nov 2012 10:50:51 +1100
Cc: Dave Chinner <dchinner@xxxxxxxxxx>, Ben Myers <bpm@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <50a5ea70.Lft3JDA+/WxpLnoh%fengguang.wu@xxxxxxxxx>
References: <50a5ea70.Lft3JDA+/WxpLnoh%fengguang.wu@xxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

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