On Feb 15, 2016, at 9:27 PM, Joe Perches wrote: > 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.
I guess they are. And I understand why you would want a comment for the actual spinlock, but pointexr - much less so. Anyway, I have some more questions: ERROR: Macros with complex values should be enclosed in parentheses #8720: FILE: drivers/staging/lustre/lustre/libcfs/tracefile.h:189: +#define cfs_tcd_for_each(tcd, i, j) \ + for (i = 0; cfs_trace_data[i]; i++) \ + for (j = 0, ((tcd) = &(*cfs_trace_data[i])[j].tcd); \ + j < num_possible_cpus(); \ + j++, (tcd) = &(*cfs_trace_data[i])[j].tcd) This is a macros with complex value alright, but the whole idea of this one is to not be enclosed. Any ideas about this one and similar?