All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Whitcroft <apw@shadowen.org>
To: "Kok, Auke" <auke-jan.h.kok@intel.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Joel Schopp <jschopp@austin.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] update checkpatch.pl to version 0.08
Date: Tue, 24 Jul 2007 10:06:51 +0100	[thread overview]
Message-ID: <46A5C12B.3080904@shadowen.org> (raw)
In-Reply-To: <46A534EA.6030008@intel.com>

Kok, Auke wrote:
> Andy Whitcroft wrote:
>> This version brings a number of new checks, and a number of bug
>> fixes.  Of note:
>>
>>   - warnings for multiple assignments per line
> 
> 
> This is bugged. e.g. the following line will hit this exception check:
> 
>     int i = some_function(a, b, c);

Yep that is plainly wrong.  Will sort that one out.

>>   - warnings for multiple declarations per line
>>   - checks for single statement blocks with braces
>>
>> This patch includes an update for feature-removal-schedule.txt to
>> better target checks.
>>
>> Andy Whitcroft (12):
>>       check for single statement braced blocks
>>
>> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
>> ---
>>
>> +# check for redundant bracing round if etc
>> +        if ($line =~ /\b(if|while|for|else)\b/) {
>> +            # Locate the end of the opening statement.
>> +            my @control = ctx_statement($linenr, $realcnt, 0);
>> +            my $nr = $linenr + (scalar(@control) - 1);
>> +            my $cnt = $realcnt - (scalar(@control) - 1);
>> +
>> +            my $off = $realcnt - $cnt;
>> +            #print "$off: line<$line>end<" . $lines[$nr - 1] . ">\n";
>> +
>> +            # If this is is a braced statement group check it
>> +            if ($lines[$nr - 1] =~ /{\s*$/) {
>> +                my ($lvl, @block) = ctx_block_level($nr, $cnt);
>> +
>> +                my $stmt = join(' ', @block);
>> +                $stmt =~ s/^[^{]*{//;
>> +                $stmt =~ s/}[^}]*$//;
>> +
>> +                #print "block<" . join(' ', @block) . "><" .
>> scalar(@block) . ">\n";
>> +                #print "stmt<$stmt>\n\n";
>> +
>> +                # Count the ;'s if there is fewer than two
>> +                # then there can only be one statement,
>> +                # if there is a brace inside we cannot
>> +                # trivially detect if its one statement.
>> +                # Also nested if's often require braces to
>> +                # disambiguate the else binding so shhh there.
>> +                my @semi = ($stmt =~ /;/g);
>> +                ##print "semi<" . scalar(@semi) . ">\n";
>> +                if ($lvl == 0 && scalar(@semi) < 2 &&
>> +                    $stmt !~ /{/ && $stmt !~ /\bif\b/) {
>> +                        my $herectx = "$here\n" . join("\n",
>> @control, @block[1 .. $#block]) . "\n";
>> +                        shift(@block);
>> +                    ERROR("braces {} are not necessary for single
>> statement blocks\n" . $herectx);
> 
> 
> This is a royal pain, since it now throws an ERROR for the obviously
> preferable piece of code below:
> 
> if (err) {
>     do_something();
>     return -ERR;
> } else {
>     do_somthing_else();
> }

Hmmm, is that obviouly nicer than the below?  Its fully a line longer
for no benefit.  But ignoring that, this seems to have snuck in to
CodingStyle hmmm ... will see what I can do if anything to stop these
being picked up I guess.

	if (err) {
		do_something();
		return -ERR;
	} else
		do_something_else();

Andrew, as you merged the change to CodingStyle I'll take that as your
being ok with these being accepted.

-apw

  parent reply	other threads:[~2007-07-24  9:07 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-15  8:25 [PATCH] update checkpatch.pl to version 0.08 Andy Whitcroft
2007-07-23 23:08 ` Kok, Auke
2007-07-24  0:11   ` Randy Dunlap
2007-07-24  9:06   ` Andy Whitcroft [this message]
2007-07-24  9:15     ` Andrew Morton
2007-07-24 11:19       ` Andy Whitcroft
2007-07-24 13:08         ` Dmitry Torokhov
2007-07-24 16:51         ` Jan Engelhardt
2007-07-24 17:20           ` Randy Dunlap
2007-07-24 17:46             ` Jan Engelhardt
2007-07-24 18:03             ` Randy Dunlap
2007-07-24 18:30               ` Andy Whitcroft
2007-07-24 17:22       ` Paul Mundt
2007-07-24 18:00         ` Jan Engelhardt
2007-07-24 18:31           ` Andy Whitcroft
2007-07-24 19:49             ` Adrian Bunk
2007-07-24 20:32               ` jschopp
2007-07-25  1:13                 ` Adrian Bunk
2007-07-25 15:39                   ` SL Baur
2007-07-25 16:54                     ` Adrian Bunk
2007-07-24 18:45         ` jschopp
2007-07-24 19:59           ` Adrian Bunk
2007-07-24 20:53             ` jschopp
2007-07-23 23:13 ` Jesper Juhl
2007-07-23 23:36 ` Kok, Auke
2007-07-24 16:53   ` Jan Engelhardt
2007-07-24 17:06     ` Andy Whitcroft
2007-08-03 12:37     ` Andy Whitcroft
2007-07-23 23:52 ` Kok, Auke
2007-07-24 11:33   ` Andy Whitcroft
2007-07-24 11:47     ` Ingo Molnar
2007-07-24 11:51       ` Ingo Molnar
2007-07-24 16:56     ` Jan Engelhardt
2007-07-24 18:38       ` Andy Whitcroft
2007-07-24 13:58   ` jschopp
2007-07-24 14:33     ` Adrian Bunk
2007-07-24 14:50       ` Andy Whitcroft

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=46A5C12B.3080904@shadowen.org \
    --to=apw@shadowen.org \
    --cc=akpm@osdl.org \
    --cc=auke-jan.h.kok@intel.com \
    --cc=jschopp@austin.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@xenotime.net \
    /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.