From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933048AbcIQV6C (ORCPT ); Sat, 17 Sep 2016 17:58:02 -0400 Received: from smtprelay0139.hostedemail.com ([216.40.44.139]:54649 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932231AbcIQV5z (ORCPT ); Sat, 17 Sep 2016 17:57:55 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::,RULES_HIT:41:355:379:541:599:960:982:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:2393:2553:2559:2562:2693:2828:2911:3138:3139:3140:3141:3142:3622:3865:3866:3867:3870:3871:3872:3873:3874:4250:4321:4425:4605:5007:7904:7974:8784:10004:10400:10848:11026:11232:11473:11657:11658:11783:11889:11914:12043:12295:12438:12679:12740:13095:13180:13229:13439:13894:14181:14659:14721:21080:21324:21433:21451:30012:30034: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:4,LUA_SUMMARY:none X-HE-Tag: ring16_44e7c8dd6a846 X-Filterd-Recvd-Size: 4315 Message-ID: <1474149472.1954.6.camel@perches.com> Subject: Re: Possible code defects: macros and precedence From: Joe Perches To: Julia Lawall Cc: Dan Carpenter , LKML Date: Sat, 17 Sep 2016 14:57:52 -0700 In-Reply-To: References: <1472927739.5018.13.camel@perches.com> <1473001581.5018.37.camel@perches.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.21.91-1ubuntu1 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 Sat, 2016-09-17 at 22:24 +0200, Julia Lawall wrote: (A 2.2MB message that (perhaps thankfully) didn't get through to lkml) > Below is the Coccinelle output for the case where the definition of the > macro is a single expression.  There is also the case where it is a > sequence of statements, but that finds very few results.  Note that > Coccinelle will only match code that it can parse, which is definitely not > always the case for macros, so some things may be missed. > > There are a huge number of results.  To the extent that you have the > patience to look through them, do you see anything undesirable? > > thanks, > julia > > diff -u -p a/lib/lz4/lz4defs.h b/lib/lz4/lz4defs.h > --- a/lib/lz4/lz4defs.h > +++ b/lib/lz4/lz4defs.h > @@ -34,7 +34,7 @@ typedef struct _U64_S { u64 v; } U64_S; >  #define PUT8(s, d) (A64(d) = A64(s)) > >  #define LZ4_READ_LITTLEENDIAN_16(d, s, p)      \ > -       (d = s - A16(p)) > +       (d = (s) - A16(p)) > >  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)        \ >         do {    \ > @@ -53,7 +53,7 @@ typedef struct _U64_S { u64 v; } U64_S; >         put_unaligned(get_unaligned((const u64 *) s), (u64 *) d) > >  #define LZ4_READ_LITTLEENDIAN_16(d, s, p)      \ > -       (d = s - get_unaligned_le16(p)) > +       (d = (s) - get_unaligned_le16(p)) > >  #define LZ4_WRITE_LITTLEENDIAN_16(p, v)                        \ >         do {                                            \ Here's the equivalent checkpatch output for that file. It has a few more instances. Is what checkpatch suggests unreasonable? $ ./scripts/checkpatch.pl -f --strict lib/lz4/lz4defs.h --types=macro_arg_precedence CHECK: Macro argument 's' may be better as '(s)' to avoid precedence issues #36: FILE: lib/lz4/lz4defs.h:36: +#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \ + (d = s - A16(p)) CHECK: Macro argument 's' may be better as '(s)' to avoid precedence issues #55: FILE: lib/lz4/lz4defs.h:55: +#define LZ4_READ_LITTLEENDIAN_16(d, s, p) \ + (d = s - get_unaligned_le16(p)) CHECK: Macro argument 'd' may be better as '(d)' to avoid precedence issues #106: FILE: lib/lz4/lz4defs.h:106: +#define LZ4_SECURECOPY(s, d, e) \ + do { \ + if (d < e) { \ + LZ4_WILDCOPY(s, d, e); \ + } \ + } while (0) CHECK: Macro argument 'e' may be better as '(e)' to avoid precedence issues #106: FILE: lib/lz4/lz4defs.h:106: +#define LZ4_SECURECOPY(s, d, e) \ + do { \ + if (d < e) { \ + LZ4_WILDCOPY(s, d, e); \ + } \ + } while (0) CHECK: Macro argument 'e' may be better as '(e)' to avoid precedence issues #147: FILE: lib/lz4/lz4defs.h:147: +#define LZ4_WILDCOPY(s, d, e) \ + do { \ + LZ4_COPYPACKET(s, d); \ + } while (d < e) CHECK: Macro argument 'l' may be better as '(l)' to avoid precedence issues #152: FILE: lib/lz4/lz4defs.h:152: +#define LZ4_BLINDCOPY(s, d, l) \ + do { \ + u8 *e = (d) + l; \ + LZ4_WILDCOPY(s, d, e); \ + d = e; \ + } while (0) total: 0 errors, 0 warnings, 6 checks, 157 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. lib/lz4/lz4defs.h has style problems, please review. NOTE: Used message types: MACRO_ARG_PRECEDENCE NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.