All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Kees Cook <keescook@chromium.org>,
	ditya Srivastava <yashsri421@gmail.com>
Cc: Andy Whitcroft <apw@canonical.com>,
	Dwaipayan Ray <dwaipayanray1@gmail.com>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH] checkpatch: Don't count URLs for "line too long"
Date: Tue, 27 Sep 2022 13:39:16 -0700	[thread overview]
Message-ID: <9de78c46ad8da9e4c59ba6206d9dcc51bc984db1.camel@perches.com> (raw)
In-Reply-To: <202209270706.DADC6346@keescook>

On Tue, 2022-09-27 at 07:07 -0700, Kees Cook wrote:
> On Mon, Sep 26, 2022 at 10:40:50PM -0700, Joe Perches wrote:
> > On Mon, 2022-09-26 at 19:15 -0700, Kees Cook wrote:
> > > URLs (not in a Link: field) should be ignored for "line too long"
> > > warnings. Allow any line containing "https://" or "http://".
> > 
> > More generally, this should be for any URI
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -3175,6 +3175,8 @@ sub process {
> > >  					# file delta changes
> > >  		      $line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ ||
> > >  					# filename then :
> > > +		      $line =~ /https?:\/\// ||
> > > +					# URLs
> > >  		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> > >  					# A Fixes: or Link: line or signature tag line
> > >  		      $commit_log_possible_stack_dump)) {
> > 
> > And I don't recollect why this wasn't applied, but I think it's a rather better,
> > more complete, patch:
> > 
> > https://lore.kernel.org/lkml/20210114073513.15773-2-yashsri421@gmail.com/
> > 
> > I believe I did ack it and forward it to Andrew Morton, but I don't
> > see it on a lore list.
> 
> Many patches have internal references in the commit log (that aren't
> appropriate for "Link:") like:
> 
> [1] https://some.domain/path/
> 
> It seems more flexible to just ignore lines with "http" in it...

URLs vs URIs which can have multiple forms.

Take a look at the results of this grep:

$ git log -1000000 --format=email --no-merges --grep='://' | grep '://' | \
  grep -v -P 'https?://' | grep -P -oh '\b\w+://' | \
  sort | uniq -c | sort -rn
    422 git://
     46 ftp://
     15 file://
     10 smb://
      5 c://
      3 usbip://
      2 svn://
      2 rsync://
      2 pvr://
      1 ttp://
      1 systemd://
      1 server://
      1 radio://
      1 nbd://
      1 irc://
      1 intel_bts://
      1 htp://
      1 40176://

The more inclusve search pattern finds many uses of git:// and ftp:// 
(and probably a few typos too)

Assuming the URI scheme uses ://, it's perhaps reasonable to change the
suggested search in Aditya's patch from

+			if ($line =~ /^\s*([a-z][\w\.\+\-]*:\/\/\S+)/i) {

to maybe something like this so the URI's use of :// is more obvious

+			if ($line =~ m@^.*\b([a-z][\w\.\+\-]*://\S+)@i) {


      reply	other threads:[~2022-09-27 20:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27  2:15 [PATCH] checkpatch: Don't count URLs for "line too long" Kees Cook
2022-09-27  5:40 ` Joe Perches
2022-09-27 14:07   ` Kees Cook
2022-09-27 20:39     ` Joe Perches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9de78c46ad8da9e4c59ba6206d9dcc51bc984db1.camel@perches.com \
    --to=joe@perches.com \
    --cc=apw@canonical.com \
    --cc=dwaipayanray1@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=yashsri421@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.