Messages in this thread | | | Date | Tue, 03 Jul 2018 12:07:51 -0700 | From | pheragu@codeauro ... | Subject | Re: [PATCH] checkpatch: Check for illegal return codes |
| |
On 2018-07-03 11:19, Joe Perches wrote: > On Tue, 2018-07-03 at 11:09 -0700, Prakruthi Deepak Heragu wrote: >> The only legal integer return is 0, anything else >> following "return" should be -ERRCODE or a function. >> >> http://lkml.org/lkml/2010/7/23/318 >> There's lots of "return -1;" statements in this patch - it's obscene >> that this is used to indicate "some error occurred" in kernel space >> rather than a real errno value - even when an existing function >> (eg, request_irq) gave you an error code already. >> >> Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org> >> Signed-off-by: Stepan Moskovchenko <stepanm@codeaurora.org> >> Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org> >> --- >> scripts/checkpatch.pl | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index a9c0550..260d252 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -6197,6 +6197,12 @@ sub process { >> "switch default: should use break\n" . $herectx); >> } >> >> +# check for return codes on error paths >> + if ($line =~ /\breturn\s+-\d+/) { >> + ERROR("NO_ERROR_CODE", >> + "illegal return value, please use an error code"); >> + } >> + > > Substitute illegal to invalid as this wouldn't be illegal. > It might be invalid and this needs a newline and $herecurr > > I'm not sure this is even useful. > > There are _way_ too many of these already existing > and simple return identifiers can be OK. > > $ git grep -P '\breturn\s*\-(?!0)\d+' | wc -l > 10193 > > and > > $ git grep -P '\breturn\s*\-(?!1)\d+' | wc -l > 240 True. However, this would be helpful to avoid usage of such return statements in the future.
| |