linux-kernel - Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking

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: <20200726153601.13855-1-sj38.park@gmail.com>
Date:   Sun, 26 Jul 2020 17:36:01 +0200
From:   SeongJae Park <sj38.park@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     SeongJae Park <sj38.park@...il.com>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        SeongJae Park <sjpark@...zon.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, apw@...onical.com,
        colin.king@...onical.com, jslaby@...e.cz, pavel@....cz,
        SeongJae Park <sjpark@...zon.de>
Subject: Re: Re: Re: Re: Re: checkpatch: support deprecated terms checking

On Sun, 26 Jul 2020 07:50:54 -0700 Joe Perches <joe@...ches.com> wrote:

> On Sun, 2020-07-26 at 09:45 +0200, SeongJae Park wrote:
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -721,6 +721,7 @@ sub read_word_corrections {
> >  my %deprecated_terms_fix;
> >  read_word_corrections($deprecated_terms_file, \%deprecated_terms_fix);
> >  my $deprecated_terms = join("|", sort keys %deprecated_terms_fix) if keys %deprecated_terms_fix;
> > +my %deprecated_terms_reported = map { $_ => 1 }
> 
> overly verbose naming and this doesn't need initialization here.
> 
> > @@ -2975,13 +2976,16 @@ sub process {
> >  		    ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) {
> >  			while ($rawline =~ /(?:^|[^a-z@])($deprecated_terms)(?:\b|$|[^a-z@])/gi) { 
> >  				my $deprecated_term = $1;
> > +				last if (exists($deprecated_terms_reported{$deprecated_term}));
> 
> next if (...) to check if multiple terms exists on the same line

Agreed on these comments, thanks!

> 
> > +				$deprecated_terms_reported{$deprecated_term} = 1;
> > +
> 
> But this does need to be reset to empty when checking the next file

Hmm... I though you mean reporting same term multiple times too verbose... Did
I misunderstand your point?

> 
> >  				my $suggested = $deprecated_terms_fix{lc($deprecated_term)};
> >  				$suggested = ucfirst($suggested) if ($deprecated_term=~ /^[A-Z]/);
> >  				$suggested = uc($suggested) if ($deprecated_term =~ /^[A-Z]+$/);
> >  				my $msg_level = \&WARN;
> >  				$msg_level = \&CHK if ($file);
> >  				if (&{$msg_level}("DEPRECATED_TERM",
> > -						  "Use of '$deprecated_term' is deprecated, please '$suggested', instead.\n" . $herecurr) &&
> > +						  "Use of '$deprecated_term' is controversial - if not required by specification, perhaps '$suggested' instead.  See: scripts/deprecated_terms.txt\n" . $herecurr) &&
> >  				    $fix) {
> >  					$fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($deprecated_term)($|[^A-Za-z@])/$1$suggested$3/;
> 
> I think it simpler to avoid emitting this on existing files.

Agreed, it's much simpler.  However, my concerns on excluding existing file
checks are:

1. Avoiding existing file checks will still not stop warning patches mentioning
existing deprecated terms.
2. If the term mistakenly comes in newly, it would be hard to check it later.
3. Some future deprecations of terms might be applied to existing uses, as
's/fuck/hug' did.

> 
> I do not want to encourage relatively inexperienced people
> to run checkpatch and submit inappropriate patches.

Me, neither.  But, I think providing more warnings and references is better for
that.  Experienced people would be able to easily ignore the false positives.
Simply limiting checks could allow people submitting inappropriate patches
intorducing new uses of deprecated terms.


Thanks,
SeongJae Park