[PATCH net-next v2] checkpatch: warn about uses of ENOTSUPP

[PATCH net-next v2] checkpatch: warn about uses of ENOTSUPP

[Jakub Kicinski]

ENOTSUPP often feels like the right error code to use, but it's
in fact not a standard Unix error. E.g.:

$ python
>>> import errno
>>> errno.errorcode[errno.ENOTSUPP]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'errno' has no attribute 'ENOTSUPP'

There were numerous commits converting the uses back to EOPNOTSUPP
but in some cases we are stuck with the high error code for backward
compatibility reasons.

Let's try prevent more ENOTSUPPs from getting into the kernel.

Recent example:
https://lore.kernel.org/netdev/20200510182252.GA411829@lunn.ch/

v2 (Joe):
 - add a link to recent discussion,
 - don't match when scanning files, not patches to avoid sudden
   influx of conversion patches.

v1:
https://lore.kernel.org/netdev/20200510185148.2230767-1-kuba@kernel.org/

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 scripts/checkpatch.pl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index eac40f0abd56..bfbdc869cd99 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4199,6 +4199,17 @@ sub process {
 			     "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr);
 		}
 
+# ENOTSUPP is not a standard error code and should be avoided in new patches.
+# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP.
+# Similarly to ENOSYS warning a small number of false positives is expected.
+		if (~$file && $line =~ /\bENOTSUPP\b/) {
+			if (WARN("ENOTSUPP",
+				 "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$fixlinenr] =~ s/\bENOTSUPP\b/EOPNOTSUPP/;
+			}
+		}
+
 # function brace can't be on same line, except for #defines of do while,
 # or if closed on same line
 		if ($perl_version_ok &&
-- 
2.25.4

Re: [PATCH net-next v2] checkpatch: warn about uses of ENOTSUPP

[Joe Perches]

On Mon, 2020-05-11 at 09:53 -0700, Jakub Kicinski wrote:
> ENOTSUPP often feels like the right error code to use, but it's
> in fact not a standard Unix error. E.g.:
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -4199,6 +4199,17 @@ sub process {
>  			     "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr);
>  		}
>  
> +# ENOTSUPP is not a standard error code and should be avoided in new patches.
> +# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP.
> +# Similarly to ENOSYS warning a small number of false positives is expected.
> +		if (~$file && $line =~ /\bENOTSUPP\b/) {

It's probably my typo or my brain thinking "not" and hitting
the tilde and not the bang, but this should be

		if (!$file & ...)

Otherwise:

Acked-by: Joe Perches <joe@perches.com>

> +			if (WARN("ENOTSUPP",
> +				 "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP\n" . $herecurr) &&
> +			    $fix) {
> +				$fixed[$fixlinenr] =~ s/\bENOTSUPP\b/EOPNOTSUPP/;
> +			}
> +		}
> +
>  # function brace can't be on same line, except for #defines of do while,
>  # or if closed on same line
>  		if ($perl_version_ok &&

Re: [PATCH net-next v2] checkpatch: warn about uses of ENOTSUPP

[Jakub Kicinski]

On Mon, 11 May 2020 10:03:31 -0700 Joe Perches wrote:
> On Mon, 2020-05-11 at 09:53 -0700, Jakub Kicinski wrote:
> > ENOTSUPP often feels like the right error code to use, but it's
> > in fact not a standard Unix error. E.g.:  
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl  
> []
> > @@ -4199,6 +4199,17 @@ sub process {
> >  			     "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr);
> >  		}
> >  
> > +# ENOTSUPP is not a standard error code and should be avoided in new patches.
> > +# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP.
> > +# Similarly to ENOSYS warning a small number of false positives is expected.
> > +		if (~$file && $line =~ /\bENOTSUPP\b/) {  
> 
> It's probably my typo or my brain thinking "not" and hitting
> the tilde and not the bang, but this should be
> 
> 		if (!$file & ...)

Ahh! :)

> Otherwise:
> 
> Acked-by: Joe Perches <joe@perches.com>

Thanks!