public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Joe Perches <joe@perches.com>
Cc: Andy Whitcroft <apw@canonical.com>, linux-kernel@vger.kernel.org
Subject: Re: [patch] checkpatch: complain about GW-BASIC style label names
Date: Thu, 7 May 2015 22:42:33 +0300	[thread overview]
Message-ID: <20150507194232.GA16501@mwanda> (raw)
In-Reply-To: <1431006471.9365.124.camel@perches.com>

On Thu, May 07, 2015 at 06:47:51AM -0700, Joe Perches wrote:
> > +#avoid GW-BASIC style label names
> > +		if ($line=~/^\+\s*(err|error|fail|out)[0-9]+:/) {
> 
> Labels aren't always only lower case.

Fair enough.  I'll test again with:

		if ($line=~/^\+\s*(err|error|fail|out)[0-9]+:/i) {

> 
> This may have false positives with ?: uses like
> 	a = foo ?
> 		err1:err2;
> 

There are no false positives.  :)

$ grep -A2 "bad label name" errs | grep ^+ | cut -b 2- | perl -ne 's/( |\t)//g; print' | cut -d : -f 1 | sort | uniq -c | sort -rn | less
    249 err1
    209 err2
    168 out1
    150 out2
    149 fail1
    132 fail2
     83 fail3
     83 err3
     78 err0
     73 out3
     70 error1
     67 error2
     48 fail0
     43 err4
     42 error0
     33 error3
     31 out4
     29 fail4
     23 out0
     22 err5
     21 error4
     14 fail5
     12 out5
     11 error5
      9 err6
      8 out6
      7 error6
      6 out7
      6 fail6
      5 out8
      5 err7
      3 error7
      2 out9
      2 error9
      2 error8
      2 err8
      2 err05
      1 out10
      1 fail9
      1 fail8
      1 fail7
      1 error10
      1 err06
      1 err04
      1 err03
      1 err01

> > +			ir (WARN("LABEL_NAME",
> > +				 "bad label name\n" . $herecurr) &&
> > +			    $fix) {
> > +				$fixed[$fixlinenr] =~
> > +				    s/^(.)\s+/$1/;
> > +			}
> > +		}
> 
> There already is a $fix option in the INDENTED_LABEL test
> above this one and isn't needed or wanted here.
> 

To be honest, I'm crap at Perl.  Give me something to cut and paste?

> It may be better to use a message like:
> 	"Prefer functionally descriptive label naming (ie: label<why>:)\n"
> 

These things are always the trickiest bit.  I think of why as the goto
location and not the label location.

WARNING: Prefer a more descriptive label.  What does the label do?\n

It's also a bit crap because labels don't do anything.

regards,
dan carpenter

  reply	other threads:[~2015-05-07 19:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07 11:21 [patch] checkpatch: complain about GW-BASIC style label names Dan Carpenter
2015-05-07 13:47 ` Joe Perches
2015-05-07 19:42   ` Dan Carpenter [this message]
2015-05-07 20:17     ` Joe Perches
2015-05-07 20:35       ` Joe Perches
2015-05-13 12:37       ` [patch v2] " Dan Carpenter
2015-05-13 13:16         ` David Sterba
2015-05-13 13:47           ` Dan Carpenter
2015-05-13 13:49           ` One Thousand Gnomes
2015-06-04 10:46             ` Dan Carpenter
2015-05-13 14:12         ` Al Viro

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=20150507194232.GA16501@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox