Re: [PATCH] checkpatch: Warn on macros with flow control statements

[Joe Perches <joe@perches.com>]

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.
 
> > 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.