Messages in this thread | | | Subject | Re: [PATCH v2] checkpatch.pl: New instances of ENOSYS are errors | From | Joe Perches <> | Date | Fri, 22 Aug 2014 10:27:01 -0700 |
| |
On Fri, 2014-08-22 at 09:26 -0700, Andy Lutomirski wrote: > ENOSYS means that a nonexistent system call was called. We have a > bad habit of using it for things like invalid operations on > otherwise valid syscalls. We should avoid this in new code. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -2931,6 +2931,14 @@ sub process { > "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); > } > > +# ENOSYS means "bad syscall nr" and nothing else > +# (note that this doesn't run on assembly files, so entry*.S is okay) > + if ($line =~ /\bENOSYS\b/) { > + ERROR("ENOSYS", > + "ENOSYS means 'invalid syscall nr' and nothing else\n" . > + " (ignore if this really is syscall entry code)\n" . $herecurr); > + } > + > # function brace can't be on same line, except for #defines of do while, > # or if closed on same line > if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
Hi again Andy.
I think this is OK, but you seem to be making this patch against an old version of checkpatch.
It applies with an offset of a few hundred lines to Linus' tree and to -next.
Some trivial comments:
I think the "(note that" comment line isn't very useful. All the tests after: next if ($realfile !~ /\.(h|c)$/); have that attribute.
checkpatch doesn't use multi-line message descriptions. The "(ignore if this" line should not be on a separate line but simply use a longer single output line.
ERROR types should not have false positives. I'd prefer WARN.
cheers, Joe
| |