Re: checkpatch falsepositives in Lustre code

[Joe Perches]

On Mon, 2016-02-15 at 20:57 -0500, Oleg Drokin wrote:
> On Feb 15, 2016, at 7:56 PM, Joe Perches wrote:
> > [etc...]
> > 
> > Yeah, that's a defect of some type.
> 
> Also while I have your attention, here's another one:
> 
> struct cfs_percpt_lock *
> cfs_percpt_lock_alloc(struct cfs_cpt_table *cptab)
> {
> ÂÂÂÂÂÂÂÂstruct cfs_percpt_lockÂÂ*pcl;
> ÂÂÂÂÂÂÂÂspinlock_tÂÂÂÂÂÂÂÂÂÂÂÂÂÂ*lock;
> ÂÂÂÂÂÂÂÂintÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂi;
> â
> ÂÂÂÂÂÂÂÂcfs_percpt_for_each(lock, i, pcl->pcl_locks)
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂspin_lock_init(lock);
> 
> The declaration of the spinlock pointer produces:
> CHECK: spinlock_t definition without comment
> 
> Should spinlock pointers really be included in the check, it's obvious that
> they themselves are not really protecting anything, esp. considering it's a
> local function variable here.

I don't have an opinion here.

spinlock_t pointers are relatively rare.

$ git grep -E "\bspinlock_t\s*\*\s*\w+\s*[=;]" | wc -l
327

~10% of them seem to have in-line comments.

$ git grep -E "\bspinlock_t\s*\*\s*\w+\s*[=;].*/\*" | wc -l
34

and just fyi, here's a top level directory breakdown:

$ git grep -E "\bspinlock_t\s*\*\s*\w+\s*[=;]" | cut -f1 -d"/" | uniq -c
ÂÂÂÂÂÂ1 Documentation
ÂÂÂÂÂ27 arch
ÂÂÂÂÂÂ1 block
ÂÂÂÂ119 drivers
ÂÂÂÂÂ24 fs
ÂÂÂÂÂ23 include
ÂÂÂÂÂÂ5 kernel
ÂÂÂÂÂÂ3 lib
ÂÂÂÂÂ67 mm
ÂÂÂÂÂ51 net
ÂÂÂÂÂÂ4 security
ÂÂÂÂÂÂ2 sound