[PATCH] checkpatch: warn about hunks which only add blank lines

[PATCH] checkpatch: warn about hunks which only add blank lines

[Denis V. Lunev]

Patches sometimes carry a hunk whose entire content is one or two
added blank lines. It changes nothing, it makes the diff longer and
it survives review because nobody looks twice at a blank line. The
blank line itself is perfectly fine, the gratuitous hunk is not.

Tally the added blank lines and the other changes of every hunk in
the pre-scan loop which already walks the raw patch, and record the
hunks which only add blanks. Such a hunk is then reported at the
first blank line it adds, so the warning carries the usual file and
line context.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Chao Liu <chao.liu@processmission.com>
CC: "Daniel P. Berrange" <berrange@redhat.com>
CC: "Philippe Mathieu-Daude" <philmd@oss.qualcomm.com>
CC: Thomas Huth <thuth@redhat.com>
---
Note: this patch is sent on behalf of the yesterday finding inside IDE
      pull request. This should be found with a checkpatch.

 scripts/checkpatch.pl | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 03f35e7501..dfa9878006 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1567,6 +1567,10 @@ sub process {
 	my $first_line = 0;
 	my $p1_prefix = '';
 
+	my %blank_only_hunk;
+	my $hunk_first_blank = 0;
+	my $hunk_has_change = 0;
+
 	my $prev_values = 'E';
 
 	# suppression flags
@@ -1583,6 +1587,10 @@ sub process {
 		$line = $rawline;
 
 		if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+			$blank_only_hunk{$hunk_first_blank} = 1
+				if ($hunk_first_blank && !$hunk_has_change);
+			($hunk_first_blank, $hunk_has_change) = (0, 0);
+
 			$realline=$1-1;
 			if (defined $2) {
 				$realcnt=$3+1;
@@ -1633,6 +1641,13 @@ sub process {
 		push(@lines, $line);
 
 		if ($realcnt > 1) {
+			if ($rawline =~ /^\+\s*$/) {
+				$hunk_first_blank = $linenr
+					if (!$hunk_first_blank);
+			} elsif ($rawline =~ /^[-+]/) {
+				$hunk_has_change = 1;
+			}
+
 			$realcnt-- if ($line =~ /^(?:\+| |$)/);
 		} else {
 			$realcnt = 0;
@@ -1641,6 +1656,8 @@ sub process {
 		#print "==>$rawline\n";
 		#print "-->$line\n";
 	}
+	$blank_only_hunk{$hunk_first_blank} = 1
+		if ($hunk_first_blank && !$hunk_has_change);
 
 	$prefix = '';
 
@@ -1780,6 +1797,10 @@ sub process {
 
 		$cnt_lines++ if ($realcnt != 0);
 
+		if ($blank_only_hunk{$linenr}) {
+			WARN("this hunk only adds blank lines\n" . $herecurr);
+		}
+
 # Only allow Python 3 interpreter
 		if ($realline == 1 &&
 			$line =~ /^\+#!\ *\/usr\/bin\/(?:env )?python$/) {

base-commit: e1705a25aff35635c360bbaba4c2731d019a422a
-- 
2.53.0

Re: [PATCH] checkpatch: warn about hunks which only add blank lines

[Thomas Huth]

On 31/07/2026 15.40, Denis V. Lunev wrote:
> Patches sometimes carry a hunk whose entire content is one or two
> added blank lines. It changes nothing, it makes the diff longer and
> it survives review because nobody looks twice at a blank line. The
> blank line itself is perfectly fine, the gratuitous hunk is not.

Well, I wouldn't say that the blank line itself is fine. It depends.
In source code, there normally should not be more than one empty line 
between code blocks, that's why I complained to the patch in your pull request.

Anyway, thanks for tackling this! But looking at the checkpatch.pl script in 
the Linux kernel, they added a slightly different check in the course of time:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=365dd4eaafa22d2c79913d5f057d636e8842c470

Maybe we should adapt that one, to avoid that the scripts diverge even further?

  Thomas

Re: [PATCH] checkpatch: warn about hunks which only add blank lines

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 31/07/2026 15.40, Denis V. Lunev wrote:
>> Patches sometimes carry a hunk whose entire content is one or two
>> added blank lines. It changes nothing, it makes the diff longer and
>> it survives review because nobody looks twice at a blank line. The
>> blank line itself is perfectly fine, the gratuitous hunk is not.
>
> Well, I wouldn't say that the blank line itself is fine. It depends.
> In source code, there normally should not be more than one empty line between code blocks, that's why I complained to the patch in your pull request.

For what it's worth, Python style *demands* two blank lines in places.
Here's what happens when I delete one of them:

    $ pycodestyle scripts/qapi/parser.py 
    scripts/qapi/parser.py:656:1: E302 expected 2 blank lines, found 1

Why?  PEP 8 "Style Guide for Python Code" section "Blank Lines":

--> Surround top-level function and class definitions with two blank
--> lines.

    Method definitions inside a class are surrounded by a single blank
    line.

    Extra blank lines may be used (sparingly) to separate groups of
    related functions. Blank lines may be omitted between a bunch of
    related one-liners (e.g. a set of dummy implementations).

    Use blank lines in functions, sparingly, to indicate logical
    sections.

    Python accepts the control-L (i.e. ^L) form feed character as
    whitespace; many tools treat these characters as page separators, so
    you may use them to separate pages of related sections of your
    file. Note, some editors and web-based code viewers may not
    recognize control-L as a form feed and will show another glyph in
    its place.

We run pycodestyle via flake8 from python/tests/linters.py.  Still not
part of "make check".

[...]

Re: [PATCH] checkpatch: warn about hunks which only add blank lines

[Denis V. Lunev]

On 8/6/26 07:28, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> On 31/07/2026 15.40, Denis V. Lunev wrote:
>>> Patches sometimes carry a hunk whose entire content is one or two
>>> added blank lines. It changes nothing, it makes the diff longer and
>>> it survives review because nobody looks twice at a blank line. The
>>> blank line itself is perfectly fine, the gratuitous hunk is not.
>> Well, I wouldn't say that the blank line itself is fine. It depends.
>> In source code, there normally should not be more than one empty line between code blocks, that's why I complained to the patch in your pull request.
> For what it's worth, Python style *demands* two blank lines in places.
> Here's what happens when I delete one of them:
>
>     $ pycodestyle scripts/qapi/parser.py 
>     scripts/qapi/parser.py:656:1: E302 expected 2 blank lines, found 1
>
> Why?  PEP 8 "Style Guide for Python Code" section "Blank Lines":
>
> --> Surround top-level function and class definitions with two blank
> --> lines.
>
>     Method definitions inside a class are surrounded by a single blank
>     line.
>
>     Extra blank lines may be used (sparingly) to separate groups of
>     related functions. Blank lines may be omitted between a bunch of
>     related one-liners (e.g. a set of dummy implementations).
>
>     Use blank lines in functions, sparingly, to indicate logical
>     sections.
>
>     Python accepts the control-L (i.e. ^L) form feed character as
>     whitespace; many tools treat these characters as page separators, so
>     you may use them to separate pages of related sections of your
>     file. Note, some editors and web-based code viewers may not
>     recognize control-L as a form feed and will show another glyph in
>     its place.
>
> We run pycodestyle via flake8 from python/tests/linters.py.  Still not
> part of "make check".
>
> [...]
>
That is actually the difference in between what is done in
the kernel and here.

There are 3 main notes:
* kernel check is not running always, it is hidden under
  --strict-only
* kernel check is not executed over Python
* kernel check catches consecutive blank lines

Here my check is different - I am trying to catch junk
hunk, which was missed with manual eye review - just
blank line. Such hunks at my opinion are absolutely
useless and that is worth to flag.

Thus kernel style check would be complementary to my
one and they are orthogonal.

I propose to stay with this version. Anyway, if you
prefer, I could port Linux one. We could try to make
voting :-)

Thank you in advance,
    Den