public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Whitcroft <apw@shadowen.org>
To: Jan Engelhardt <jengelh@computergmbh.de>
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.05
Date: Sun, 17 Jun 2007 01:44:37 +0100	[thread overview]
Message-ID: <467483F5.8020706@shadowen.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0706162353390.32724@fbirervta.pbzchgretzou.qr>

Jan Engelhardt wrote:
> On Jun 16 2007 22:42, Andy Whitcroft wrote:
>> @@ -180,12 +182,17 @@ sub ctx_block_get {
>> sub ctx_block_outer {
>> 	my ($linenr, $remain) = @_;
>>
>> -	return ctx_block_get($linenr, $remain, 1);
>> +	return ctx_block_get($linenr, $remain, 1, '\{', '\}');
> 
> '\\{'.

I want the string to be \{ ... '\}' gives me that:

$ perl
$q = '\}';
print "$q\n";
\}

> Or, if it works, directly use
> 	return &ctx_block_get($linenr, $remain, 1, qr/\{/, qr/\}/);
> 
>> +sub ctx_statement {
>> +	my ($linenr, $remain) = @_;
>> +
>> +	return ctx_block_get($linenr, $remain, 0, '\(', '\)');
> 
> ^^
> 
>> +	my $ident	= '[A-Za-z\d_]+';
> 
> Oh yes, use the qr operator here. (qr{}, qr//, choose anything like you
> would do with m//)

Well I want a combination of variable expanded and not expanded.  I
think there will be a general cleanup to some standard quoting for the
RE's as there are hundreds, and about 7 different quote styles right now.

> 
>> +	my $storage	= '(?:extern|static)';
>> +	my $sparse	= '(?:__user|__kernel|__force|__iomem)';
>> +	my $type	= '(?:unsigned\s+)?' .
>> +			  '(?:void|char|short|int|long|unsigned|float|double|' .
>> +			  'long\s+long|' .
>> +			  "struct\\s+${ident}|" .
>> +			  "union\\s+${ident}|" .
>> +			  "${ident}_t)" .
>> +			  "(?:\\s+$sparse)*" .
>> +			  '(?:\s*\*+)?';
>> +	my $attribute	= '(?:__read_mostly|__init|__initdata)';
>> +
>> +	my $Ident	= $ident;
>> +	my $Type	= $type;
>> +	my $Storage	= $storage;
>> +	my $Declare	= "(?:$storage\\s+)?$type";
>> +	my $Attribute	= $attribute;
>> +
> 
>> #trailing whitespace
>> -		if ($line=~/\+.*\S\s+$/) {
>> +		if ($line=~/^\+.*\S\s+$/) {
>                 if ($line =~ /^\+.*\S\s+$/) {
>> 			my $herevet = "$here\n" . cat_vet($line) . "\n\n";
>> 			print "trailing whitespace\n";
>> 			print "$herevet";
>> @@ -392,17 +420,20 @@ sub process {
>> 		#
>> 		next if ($in_comment);
>>
>> -		# Remove comments from the line before processing.
>> +# Remove comments from the line before processing.
>> 		$line =~ s@/\*.*\*/@@g;
>> 		$line =~ s@/\*.*@@;
> 
> C being a wonderful language, has this nice pitfall for parsers
> 
> 	foo = number /*pointer_to_int;

Hmm really?  Thats not how gcc seems to parse that, it seems to think
its a comment.  Which makes us safe, as the compiler will trip them up.

$ cat test4.c
int main(int argc, char *argv[])
{
        int _p = 10;
        int *p = &_p;

        int foo = 10 /*p;

        printf("foo=%d\n", foo);
}
$ cc -o test4 test4.c
test4.c:6:15: error: unterminated comment
test4.c: In function 'main':
test4.c:6: error: expected ',' or ';' at end of input
test4.c:6: error: expected declaration or statement at end of input

>> 		$line =~ s@.*\*/@@;
>>
>> -		#
>> -		# Checks which may be anchored in the context.
>> -		#
>> +# Standardise the strings and chars within the input to simplify matching.
>> +		$line = sanitise_line($line);
>> +
>> +#
>> +# Checks which may be anchored in the context.
>> +#
>>
>> -		# Check for switch () and associated case and default
>> -		# statements should be at the same indent.
>> +# Check for switch () and associated case and default
>> +# statements should be at the same indent.
>> 		if ($line=~/\bswitch\s*\(.*\)/) {
> 
> Codingstyle warrants \bswitch\s+  :)

Yep and we check for that.  But here we are trying to catch a switch and
case at differing levels, we want to catch that whether they got their
spacing right or not.

>> # * goes on variable not on type
>> -		my $type = '(?:char|short|int|long|unsigned|float|double|' .
>> -			   'struct\s+[A-Za-z\d_]+|' .
>> -			   'union\s+[A-Za-z\d_]+)';
>> -
> 
> qr. (I don't know what it is good for - compare qr/xyz/ with 'xyz'...,
> but there's a reason to its existence, so let's use it :-)

Well I would tend to say use what works and is easy to understand.  The
semantics of '' and "" are well known, to change to qr{} I would have to
go read the manual to know what it is going to do.  That said, _if_ it
did have the same semantics as m// then it may wel allow all of these to
be expressed as qr{} as it would expand variables but treat \ as if we
were in ''.  So ... to the manual for me.

-apw

      reply	other threads:[~2007-06-17  0:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-16 21:42 [PATCH] update checkpatch.pl to version 0.05 Andy Whitcroft
2007-06-16 22:00 ` Jan Engelhardt
2007-06-17  0:44   ` Andy Whitcroft [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=467483F5.8020706@shadowen.org \
    --to=apw@shadowen.org \
    --cc=akpm@osdl.org \
    --cc=jengelh@computergmbh.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox