linux-kernel - Re: [PATCH 1/1] checkpatch: check for subject uniqueness in git repository.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1410814924.18787.37.camel@joe-AO725>
Date:	Mon, 15 Sep 2014 14:02:04 -0700
From:	Joe Perches <joe@...ches.com>
To:	Fabian Frederick <fabf@...net.be>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/1] checkpatch: check for subject uniqueness in git
 repository.

On Mon, 2014-09-15 at 20:43 +0200, Fabian Frederick wrote:
> Adding patch subject uniqueness check in checkpatch --strict mode.
> See Documentation/SubmittingPatches/globally-unique identifier.

I'm not sold on the concept as I'm not sure that
global subject uniqueness is all that important.

There are many "update defconfig" type patches that
have identical subjects.  There really isn't a need
to have more information or add a numeric sequence #
to a subject like that.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> +# Check patch subject on subjective/strict check mode
> +		if ($check && $line =~ /^Subject: \[(.*)\](.*)/) {
> +			my $subject = $2;
> +			if ($quiet == 0) {
> +				print "Looking for patches with the same subject in git repository ...\n";
> +			}
> +			$git_samesubjects = `git log --oneline | grep -m1 "$subject\$" | wc  -l`;

checkpatch doesn't require a git repository to be
successfully used.

Please add a new function, something like the existing
"sub git_commit_info", for this so that if git is not
installed, there will not be a runtime failure.

Store the output in a perl array and use perl's grep
mechanism rather than the grep command line tool.

This will be _very_ slow.

On my computer with an ssd it takes > 20 seconds
just to do:

$ git --no-pager log --nocolor --no-merges --format='%H %s'

fyi: there are already > ~3500 commits with duplicate titles.

This will not find duplicate titles in a patch series unless
the patches were created with git and the current branch is
the same one that was used to create the series.

$ git checkout -b <name>
while (more changes to make) {
  [ modify / compile / test]
  $ git commit ...
}
$ git format-patch -o <dir> --cover-letter master
$ git checkout master
$ ./scripts/checkpatch.pl <dir>/*

No problem would be found...

Now for a patch series, it would be useful/better to scan
directories for patches with duplicate titles using
something like:

$ ./scripts/checkpatch.pl <dir>/*.patch

and compare the "^Subject:" lines for duplicates without
using git at all.

> @@ -5091,6 +5101,10 @@ sub process {
>  		ERROR("MISSING_SIGN_OFF",
>  		      "Missing Signed-off-by: line(s)\n");
>  	}
> +	if ($is_patch && $git_samesubjects > 0)  {
> +		WARN("NOT_UNIQUE_SUBJECT",
> +		     "similar subjects found in git repository\n");

s/similar subjects/identical patch subjects/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/