Re: "CodingStyle: Clarify and complete chapter 7" in docs-next

From: Jean Delvare
Date: Thu Sep 22 2016 - 07:58:10 EST


On Thu, 22 Sep 2016 03:42:10 -0700, Joe Perches wrote:
> On Thu, 2016-09-22 at 11:24 +0200, Jean Delvare wrote:
> > I would rather suggest:
> >
> > ERROR -> MUST_FIX
> > WARNING -> SHOULD_FIX
> > CHECK -> MAY_FIX
>
> MUST is much stronger language than I would prefer.

That's what error means, really. When your compiler fails with an
error, you have no choice but to fix your code. Warnings on the other
hand may be ignored sometimes.

> There are still about a quarter million ERRORs just for
> spacing issues in the kernel tree.
>
> Here are the top 10 ERROR checkpatch messages treewide as of
> a few days ago,
>
> $ grep ERROR checkpatch.short_sorted_20160917
> 268308 ERROR:SPACING
> 37340 ERROR:CODE_INDENT
> 27678 ERROR:TRAILING_WHITESPACE
> 21024 ERROR:COMPLEX_MACRO
> 14048 ERROR:POINTER_LOCATION
> 12207 ERROR:TRAILING_STATEMENTS
> 11079 ERROR:OPEN_BRACE
> 6802 ERROR:ASSIGN_IN_IF
> 3940 ERROR:RETURN_PARENTHESES
> 2322 ERROR:NON_OCTAL_PERMISSIONS
>
> Maybe there could be some better classifications of the various
> messages.
>
> But there are about two million checkpatch messages overall in
> the kernel tree.
>
> That's a lot.

Sure. But I'm afraid you keep changing topics and I have no idea where
you are going. We started with "should there be a space before jump
labels", then out of nowhere we were discussing the wording of the
output of checkpatch (how is that related?) and now you pull statistics
out of your hat, like these numbers imply anything.

checkpatch was called checkPATCH for a reason. It's main intent was to
prevent NEW (coding-style mostly) errors from creeping into the kernel.
The fact that old code does now always follow these recommendations is
unfortunate but that doesn't make checkpatch wrong or bad.

ERROR means that the new code isn't allowed to do that. Period.

--
Jean Delvare
SUSE L3 Support