Messages in this thread | | | Subject | Re: [PATCH] checkpatch: Check for illegal return codes | From | Joe Perches <> | Date | Tue, 03 Jul 2018 12:37:24 -0700 |
| |
On Tue, 2018-07-03 at 12:07 -0700, pheragu@codeaurora.org wrote: > 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. [] > > > diff --git 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.
Maybe use CHK and not ERROR and maybe the output message could be something like "Prefer 'return -<APPROPRIATE_ERRNO>'"
| |