From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753370AbcBPDFa (ORCPT ); Mon, 15 Feb 2016 22:05:30 -0500 Received: from smtprelay0154.hostedemail.com ([216.40.44.154]:44529 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752572AbcBPDF2 (ORCPT ); Mon, 15 Feb 2016 22:05:28 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::,RULES_HIT:41:152:355:379:541:599:960:973:982:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1543:1593:1594:1711:1730:1747:1777:1792:2393:2538:2553:2559:2562:2692:2893:2897:2898:3138:3139:3140:3141:3142:3355:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4321:5007:6119:6261:7875:7903:9010:10004:10400:10848:11026:11232:11658:11783:11889:11914:12043:12295:12438:12517:12519:12679:12740:13894:14659:21080:30012:30054:30070:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: word53_86d89aa35e043 X-Filterd-Recvd-Size: 4612 Message-ID: <1455591925.4046.40.camel@perches.com> Subject: Re: checkpatch falsepositives in Lustre code From: Joe Perches To: Oleg Drokin Cc: Andy Whitcroft , LKML , Andrew Morton Date: Mon, 15 Feb 2016 19:05:25 -0800 In-Reply-To: References: <1E5E2198-2E5C-4B6F-AAA5-C28E0A776714@linuxhacker.ru> <1455584189.4046.28.camel@perches.com> <073E66B1-D39F-4D03-BDC7-68B18172BA5D@linuxhacker.ru> <1455589627.4046.35.camel@perches.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.4-0ubuntu1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2016-02-15 at 21:45 -0500, Oleg Drokin wrote: > 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? checkpatch is brainless script and a not a real parser. Ignoring its stupid and incorrect messages is a good idea. fyi: There are many of these messages that exist like below. I can't think of a reasonable way to automatically identify and not show the defective error messages for these. Andy? --- ERROR: Macros with complex values should be enclosed in parentheses #86: FILE: include/linux/dmar.h:86: +#define for_each_active_drhd_unit(drhd) \ + list_for_each_entry_rcu(drhd, &dmar_drhd_units, list) \ + if (drhd->ignored) {} else ERROR: Macros with complex values should be enclosed in parentheses #90: FILE: include/linux/dmar.h:90: +#define for_each_active_iommu(i, drhd) \ + list_for_each_entry_rcu(drhd, &dmar_drhd_units, list) \ + if (i=drhd->iommu, drhd->ignored) {} else ERROR: Macros with complex values should be enclosed in parentheses #94: FILE: include/linux/dmar.h:94: +#define for_each_iommu(i, drhd) \ + list_for_each_entry_rcu(drhd, &dmar_drhd_units, list) \ + if (i=drhd->iommu, 0) {} else  ERROR: Macros with complex values should be enclosed in parentheses #110: FILE: include/linux/dmar.h:110: +#define for_each_active_dev_scope(a, c, p, d) \ + for_each_dev_scope((a), (c), (p), (d)) if (!(d)) { continue; } else total: 4 errors, 0 warnings, 285 lines checked