Re: [Qemu-devel] [PATCH 2/4] CODING_STYLE, checkpatch: update line length rules

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> Line lengths above 80 characters do exist.  They are rare, but
> they happen from time to time.  An ignored rule is worse than an
> exception to the rule, so do the latter.
>
> Based on remarks from the list, make the preferred line length
> slightly lower than 80 characters, to account for extra characters
> in unified diffs (including three-way diffs) and for email quoting.
>
> Checkpatch has some code to detect doc comments that doesn't apply
> to QEMU; the usual limits apply even for doc comments in our case.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  CODING_STYLE          | 13 ++++++++++---
>  scripts/checkpatch.pl | 21 +++++++++++++++------
>  2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 3c6978f..34d5526 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -31,14 +31,21 @@ Do not leave whitespace dangling off the ends of lines.
>  
>  2. Line width
>  
> -Lines are 80 characters; not longer.
> +Lines should be 76 characters; try not to make them longer.
> +
> +Sometimes it is hard to do, especially when dealing with QEMU subsystems
> +that use long function or symbol names.  Even in that case, do not make
> +lines much longer than 76 characters.
>  
>  Rationale:
>   - Some people like to tile their 24" screens with a 6x4 matrix of 80x24
> -   xterms and use vi in all of them.  The best way to punish them is to
> -   let them keep doing it.
> +   xterms and use vi in all of them.  They also examine diffs (and three-way
> +   diffs) on an 80-column terminal, accounting for two extra characters.
> +   The best way to punish them is to let them keep doing it.
>   - Code and especially patches is much more readable if limited to a sane
>     line length.  Eighty is traditional.
> + - The four-space indentation makes the most common excuse ("But look
> +   at all that white space on the left!") moot.
>   - It is the QEMU coding style.
>  
>  3. Naming
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7f0aae9..bc32d8f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1466,14 +1466,23 @@ sub process {
>  # check we are in a valid source file if not then ignore this hunk
>               next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
>  
> -#80 column limit
> -             if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
> -                 $rawline !~ /address@hidden/ &&
> +#90 column limit
> +             if ($line =~ /^\+/ &&
>                   !($line =~ 
> /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:,|\)\s*;)\s*$/
>  ||
> -                 $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
> -                 $length > 80)
> +                 !($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/))
>               {
> -                     WARN("line over 80 characters\n" . $herecurr);
> +                     if ($length > 90) {
> +                             ERROR("line over 90 characters\n" . $herecurr);
> +                     } elsif ($length > 76 && !($rawline =~ /^\+ \* /)) {
> +                             # The BSD license blurb has 80 character lines.
> +                             # Avoid warning on cut-and-pasted license text.

Why not simply reflow all the offending license blurbs?

Want me to prep such a patch?

> +                             WARN("line over 76 characters\n" . $herecurr);
> +                     } elsif ($length > 80) {
> +                             # Do not confuse the user by introducing yet
> +                             # another limit (80 characters).  Technically,
> +                             # this line *is* over 76 characters.
> +                             WARN("line over 76 characters\n" . $herecurr);
> +                     }
>               }
>  
>  # check for spaces before a quoted newline