Messages in this thread | | | Date | Thu, 1 Oct 2015 13:47:18 -0400 | From | Johan Hovold <> | Subject | Re: [PATCH] coccinelle: misc: remove "complex return code" warnings |
| |
On Thu, Oct 01, 2015 at 07:20:10AM +0200, Julia Lawall wrote: > On Wed, 30 Sep 2015, Johan Hovold wrote: > > > This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch > > to delete overly complex return code processing"). > > > > There can be both symmetry and readability reasons for not wanting to do > > the final function call as part of the return statement and to maintain > > a clear separation of success and error paths. > > > > Since this is in no way mandated by the coding standard, let's just > > remove this semantic patch to avoid having "clean up" patches being > > posted over and over in response to these Coccinelle warnings. > > What do you mean by "posted"? Are you referring to 0-day build testing > or individual usage of make coccicheck? Maybe it would make sense to > remove the semantic patch from 0-day build testing but leave it in the > kernel, perhaps removing the < 0 case because that one in practice doesn't > seem to turn up much that is useful?
Individuals running coccicheck on in-kernel code and posting patches to "fix warnings", where the end result is not necessarily an improvement.
But I don't think these warnings should be enabled for 0-day build testing either as it is should be up to the author to decide what style to prefer in each case.
> Perhaps it could also be improved to detect a previous != 0 case and then > not return a warning. On some functions, this change can make some nice > simplifications.
Yes, that would at least improve things.
I don't think warnings should be generated at all for the following code:
{ int ret;
ret = init_a(...); if (ret) return ret;
ret = init_b(...); if (ret) return ret;
return 0; }
as it is (at least to me) preferred over:
{ int ret;
ret = init_a(...); if (ret) return ret;
return init_b(...); }
for symmetry and readability reasons (e.g. I don't have to look at init_b to figure out what the functions returns). And with a long parameter list to init_b with line breaks, this would look even worse.
But either way, it should be up to the author of the code to decide what style to use.
Thanks, Johan
| |