Hi Dave, > > + 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. OK, thanks. > > 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. Ah OK, that macro does confuse sparse.. > 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. Fair enough. However I'd like to do it in a slightly different way. The problem is that there are lots of existing (ie. old) valid warnings on the missing "static". I'd still like the auto generated patch to fix these old ones by the way. At the same time, to avoid the *duplicated* chunks, I'll tell the script to remember the list of symbols that have been made static by the generated patches. This should address your concern, while still be able to gradually get rid of the existing static warnings. > For that reason, I think I'd prefer it if your build bot > just sent build warnings, not patches. I think the patches could be improved rather than removed. I'll fix the duplicated patches like in this case. Since there tend to be lots of "Should it be static?" warnings, it would save some (boring) human time by providing an auto generated patch for consideration. > 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? Yes, the build script has code to avoid sending reports on interim warnings. It will double check the branch HEAD and if the warnings to be reported have disappeared there, nothing will be reported. However if it's a build *error* fixed in the HEAD, it's a potential problem for bisection, so will still be reported unless it's a known tree that won't do rebase (ie. net/master). Thanks, Fengguang _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
