From: Julia Lawall <julia.lawall@lip6.fr> To: Joe Perches <joe@perches.com> Cc: Dan Carpenter <dan.carpenter@oracle.com>, Andrew Morton <akpm@linux-foundation.org>, devel@driverdev.osuosl.org, "Dilger, Andreas" <andreas.dilger@intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Peng Tao <bergwolf@gmail.com>, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, "Drokin, Oleg" <oleg.drokin@intel.com>, Julia Lawall <Julia.Lawall@lip6.fr>, Andy Whitcroft <apw@canonical.com> Subject: Re: [PATCH] checkpatch: Warn on macros with flow control statements Date: Wed, 10 Sep 2014 16:06:01 +0200 (CEST) [thread overview] Message-ID: <alpine.DEB.2.10.1409101604210.4747@hadrien> (raw) In-Reply-To: <1410357331.24028.39.camel@joe-AO725> On Wed, 10 Sep 2014, Joe Perches wrote: > On Wed, 2014-09-10 at 11:43 +0300, Dan Carpenter wrote: > > On Tue, Sep 09, 2014 at 01:38:13PM -0700, Joe Perches wrote: > > > Macros with flow control statements (goto and return) are > > > not very nice to read as any flow movement is unexpected. > > break and continue are also flow control statements > but are those are frequently used in macros in > complete switch statements so were not added. Would it be possible to make a warning when there is a break or continue but no while/switch/etc. julia > > > Try to highlight them and emit a warning on their definition. > > > > > > Avoid warning on macros that use argument concatenation as > > > those macros commonly create another function where the > > > concatenation is used in the function name definition like: > > > #define FOO_FUNC(name, rtn_type) \ > > > rtn_type func##name(arg1, ...) \ > > > { \ > > > rtn_type rtn; \ > > > [code...] \ > > > return rtn; \ > > > } > > > > > > > It adds 382 new warnings. > > Thanks for running it over the tree. > > > The '##' trick doesn't remove all then macros which create functions. > > I can't think of a better way to do that though. > > Nor I. I suppose it could be a --strict CHK and not > a WARN message type though. > > > We will eventually get rid of almost all the warnings in staging. The > > one that makes sense to keep is: > > > > drivers/staging/lustre/lnet/selftest/selftest.h:559 > > #define STATE2STR(x) case x: return #x > > Yup, there are a few of those and they should > definitely stay. > > > My guess is that other maintainers won't be as excited to change these... > > Do maintainers ever get excited about style? > > > Some of the macros have "RETURN", "RET" or "EXIT" in the name so the > > return is not really hidden. > > Not sure what to do about that. > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
WARNING: multiple messages have this Message-ID (diff)
From: Julia Lawall <julia.lawall@lip6.fr> To: Joe Perches <joe@perches.com> Cc: Dan Carpenter <dan.carpenter@oracle.com>, Andrew Morton <akpm@linux-foundation.org>, devel@driverdev.osuosl.org, "Dilger, Andreas" <andreas.dilger@intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Peng Tao <bergwolf@gmail.com>, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, "Drokin, Oleg" <oleg.drokin@intel.com>, Julia Lawall <Julia.Lawall@lip6.fr>, Andy Whitcroft <apw@canonical.com> Subject: Re: [PATCH] checkpatch: Warn on macros with flow control statements Date: Wed, 10 Sep 2014 14:06:01 +0000 [thread overview] Message-ID: <alpine.DEB.2.10.1409101604210.4747@hadrien> (raw) In-Reply-To: <1410357331.24028.39.camel@joe-AO725> On Wed, 10 Sep 2014, Joe Perches wrote: > On Wed, 2014-09-10 at 11:43 +0300, Dan Carpenter wrote: > > On Tue, Sep 09, 2014 at 01:38:13PM -0700, Joe Perches wrote: > > > Macros with flow control statements (goto and return) are > > > not very nice to read as any flow movement is unexpected. > > break and continue are also flow control statements > but are those are frequently used in macros in > complete switch statements so were not added. Would it be possible to make a warning when there is a break or continue but no while/switch/etc. julia > > > Try to highlight them and emit a warning on their definition. > > > > > > Avoid warning on macros that use argument concatenation as > > > those macros commonly create another function where the > > > concatenation is used in the function name definition like: > > > #define FOO_FUNC(name, rtn_type) \ > > > rtn_type func##name(arg1, ...) \ > > > { \ > > > rtn_type rtn; \ > > > [code...] \ > > > return rtn; \ > > > } > > > > > > > It adds 382 new warnings. > > Thanks for running it over the tree. > > > The '##' trick doesn't remove all then macros which create functions. > > I can't think of a better way to do that though. > > Nor I. I suppose it could be a --strict CHK and not > a WARN message type though. > > > We will eventually get rid of almost all the warnings in staging. The > > one that makes sense to keep is: > > > > drivers/staging/lustre/lnet/selftest/selftest.h:559 > > #define STATE2STR(x) case x: return #x > > Yup, there are a few of those and they should > definitely stay. > > > My guess is that other maintainers won't be as excited to change these... > > Do maintainers ever get excited about style? > > > Some of the macros have "RETURN", "RET" or "EXIT" in the name so the > > return is not really hidden. > > Not sure what to do about that. > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
next prev parent reply other threads:[~2014-09-10 14:06 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-09-07 16:18 [PATCH 6/7] staging: lustre: obdclass: expand the GOTO macro + break Julia Lawall 2014-09-07 16:18 ` Julia Lawall 2014-09-09 12:54 ` Dan Carpenter 2014-09-09 12:54 ` Dan Carpenter 2014-09-09 13:05 ` Julia Lawall 2014-09-09 13:05 ` Julia Lawall 2014-09-09 13:37 ` Drokin, Oleg 2014-09-09 13:37 ` Drokin, Oleg 2014-09-09 20:38 ` [PATCH] checkpatch: Warn on macros with flow control statements Joe Perches 2014-09-09 20:38 ` Joe Perches 2014-09-10 8:43 ` Dan Carpenter 2014-09-10 8:43 ` Dan Carpenter 2014-09-10 13:55 ` Joe Perches 2014-09-10 13:55 ` Joe Perches 2014-09-10 14:06 ` Julia Lawall [this message] 2014-09-10 14:06 ` Julia Lawall 2014-09-10 14:36 ` Joe Perches 2014-09-10 14:36 ` Joe Perches 2014-09-10 14:52 ` Drokin, Oleg
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.10.1409101604210.4747@hadrien \ --to=julia.lawall@lip6.fr \ --cc=akpm@linux-foundation.org \ --cc=andreas.dilger@intel.com \ --cc=apw@canonical.com \ --cc=bergwolf@gmail.com \ --cc=dan.carpenter@oracle.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=joe@perches.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=oleg.drokin@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.