linux-kernel - Re: [patch] checkpatch: complain about GW-BASIC style label names

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: <20150507194232.GA16501@mwanda>
Date:	Thu, 7 May 2015 22:42:33 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Andy Whitcroft <apw@...onical.com>, linux-kernel@...r.kernel.org
Subject: Re: [patch] checkpatch: complain about GW-BASIC style label names

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
--
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/