Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Christer Weinigel
On Thu, 03 Jan 2008 17:17:29 +0200
Benny Halevy <[EMAIL PROTECTED]> wrote:

> On Jan. 03, 2008, 14:30 +0200, Arnaldo Carvalho de Melo
> > Agreed, CodingStyle is not about mindless consistency such as "for
> > (" is the right thing, so "list_for_each (" is consistent with it,
> > it is about codifying practice contributors got used to over the
> > years.
> > 
> 
> Why mindless?
> Coding style is also about giving the coding language logic a
> graphical representation.  Following a convention that flow control
> keywords such as "if", "for", or "while" are distinguished from
> function calls by use of a space after the keyword really helps the
> code readability regardless of how people used to code it in the
> past... The for_each_* macros are clearly not function calls but
> rather translate to for () flow control constructs hence they should
> follow the same convention. FWIW, I think that changing the existing
> convention is worth it in this case.

Definite agreement here, since _for_each is used for flow control, that
space should be there.  

And some people seem to take checkpatch.pl as the gospel, and won't
apply code with checkpatch warnings.

  /Christer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Christer Weinigel
On Thu, 03 Jan 2008 13:34:45 +0100
Tomas Carnecky <[EMAIL PROTECTED]> wrote:

> Christer Weinigel wrote:
> > By the way, what is the consensus on lines over 80 characters?
> > checkpatch complains about the following:
> > 
> > WARNING: line over 80 characters
> > #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
> > +   printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav
> > Technologies AB\n");
> > 
> > I can of course break this into:
> > 
> > printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav "
> >"Technologies AB\n");
> > 
> > but in my opinion that becomes more even unreadable.  Would it be
> > possible to add a special case so that checkpatch ignores long
> > strings that go beyond 80 characters?  Do you think it is a good
> > idea?
> 
> At the top of the file add a #define and use that in the code? Some 
> drivers define their version/author etc that way and then just
> printk(DRIVER_VERSION DRIVER_AUTHOR);

That only solves this specific problem.  For debugging printks, which
often become quite wide, it would make the code even more unreadable to
add lots of defines just to keep things within 80 cols.

  /Christer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Benny Halevy
On Jan. 03, 2008, 14:30 +0200, Arnaldo Carvalho de Melo <[EMAIL PROTECTED]> 
wrote:
> Em Thu, Jan 03, 2008 at 12:26:10PM +, Christoph Hellwig escreveu:
>> On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
>>> We have had some stabs at changing this, but no consensus was reached on
>>> whether it was a "for" or a "function".  My memory is of there being
>>> slightly more "without a space" tenders than the other and so it has not
>>> been changed.  This thread also seems so far to have not really
>>> generated a concensus.  So I would tend to leave things as they are.  
>>>
>>> A third option might be to accept either on *for_each* constructs.
>>> That might tend to lead to divergance.  Difficult.  However, also see my
>>> later comments on "style guide".
>> Pretty much all core code uses list_for_each_entry( so new code should
>> follow that example.
> 
> Agreed, CodingStyle is not about mindless consistency such as "for (" is
> the right thing, so "list_for_each (" is consistent with it, it is about
> codifying practice contributors got used to over the years.
> 

Why mindless?
Coding style is also about giving the coding language logic a graphical
representation.  Following a convention that flow control keywords
such as "if", "for", or "while" are distinguished from function calls
by use of a space after the keyword really helps the code readability
regardless of how people used to code it in the past...
The for_each_* macros are clearly not function calls but rather translate
to for () flow control constructs hence they should follow the same convention.
FWIW, I think that changing the existing convention is worth it in this case.

Benny

> - Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Tomas Carnecky

Christer Weinigel wrote:

By the way, what is the consensus on lines over 80 characters?
checkpatch complains about the following:

WARNING: line over 80 characters
#762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
+   printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies 
AB\n");

I can of course break this into:

printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav "
   "Technologies AB\n");

but in my opinion that becomes more even unreadable.  Would it be
possible to add a special case so that checkpatch ignores long strings
that go beyond 80 characters?  Do you think it is a good idea?


At the top of the file add a #define and use that in the code? Some 
drivers define their version/author etc that way and then just

printk(DRIVER_VERSION DRIVER_AUTHOR);

tom
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Arnaldo Carvalho de Melo
Em Thu, Jan 03, 2008 at 12:26:10PM +, Christoph Hellwig escreveu:
> On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
> > We have had some stabs at changing this, but no consensus was reached on
> > whether it was a "for" or a "function".  My memory is of there being
> > slightly more "without a space" tenders than the other and so it has not
> > been changed.  This thread also seems so far to have not really
> > generated a concensus.  So I would tend to leave things as they are.  
> > 
> > A third option might be to accept either on *for_each* constructs.
> > That might tend to lead to divergance.  Difficult.  However, also see my
> > later comments on "style guide".
> 
> Pretty much all core code uses list_for_each_entry( so new code should
> follow that example.

Agreed, CodingStyle is not about mindless consistency such as "for (" is
the right thing, so "list_for_each (" is consistent with it, it is about
codifying practice contributors got used to over the years.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Christoph Hellwig
On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
> We have had some stabs at changing this, but no consensus was reached on
> whether it was a "for" or a "function".  My memory is of there being
> slightly more "without a space" tenders than the other and so it has not
> been changed.  This thread also seems so far to have not really
> generated a concensus.  So I would tend to leave things as they are.  
> 
> A third option might be to accept either on *for_each* constructs.
> That might tend to lead to divergance.  Difficult.  However, also see my
> later comments on "style guide".

Pretty much all core code uses list_for_each_entry( so new code should
follow that example.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread pHilipp Zabel
On Dec 2, 2007 1:03 PM, Christer Weinigel <[EMAIL PROTECTED]> wrote:
> Hi Andy,
>
> you seem to be the last person messing around with checkpatch.pl so I'm
> addressing this to you. :-)
>
> checkpatch complains about the following:
>
> WARNING: no space between function name and open parenthesis '('
> #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
> +   list_for_each_entry (transfer, >transfers, transfer_list) {
>
> which I think is a bit bogus since it actually is a for statement in
> disguise.  The following patch adds list_for_each to the list of things
> that look like functions that it shouldn't complain about.
>
> By the way, what is the consensus on lines over 80 characters?
> checkpatch complains about the following:
>
> WARNING: line over 80 characters
> #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
> +   printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav 
> Technologies AB\n");
>
> I can of course break this into:
>
> printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav "
>"Technologies AB\n");

I'd not split it in the middle of a name or phrase if possible.
printk(KERN_INFO "S3C24xx SPI DMA driver"
"(c) 2007 Nordnav Technologies AB\n");
but ...

> but in my opinion that becomes more even unreadable.  Would it be
> possible to add a special case so that checkpatch ignores long strings
> that go beyond 80 characters?  Do you think it is a good idea?

... in this case
   pr_info("S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies AB\n");
might just push it under the limit.

cheers
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Andy Whitcroft
On Sun, Dec 02, 2007 at 01:03:35PM +0100, Christer Weinigel wrote:
> Hi Andy,
> 
> you seem to be the last person messing around with checkpatch.pl so I'm
> addressing this to you. :-)
> 
> checkpatch complains about the following:
> 
> WARNING: no space between function name and open parenthesis '('
> #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
> +   list_for_each_entry (transfer, >transfers, transfer_list) {
> 
> which I think is a bit bogus since it actually is a for statement in
> disguise.  The following patch adds list_for_each to the list of things
> that look like functions that it shouldn't complain about.

We have had some stabs at changing this, but no consensus was reached on
whether it was a "for" or a "function".  My memory is of there being
slightly more "without a space" tenders than the other and so it has not
been changed.  This thread also seems so far to have not really
generated a concensus.  So I would tend to leave things as they are.  

A third option might be to accept either on *for_each* constructs.
That might tend to lead to divergance.  Difficult.  However, also see my
later comments on "style guide".

> 
> By the way, what is the consensus on lines over 80 characters?
> checkpatch complains about the following:
> 
> WARNING: line over 80 characters
> #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
> +   printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav 
> Technologies AB\n");
> 
> I can of course break this into:
> 
> printk(KERN_INFO "S3C24xx SPI DMA driver (c) 2007 Nordnav "
>  "Technologies AB\n");
> 
> but in my opinion that becomes more even unreadable.  Would it be
> possible to add a special case so that checkpatch ignores long strings
> that go beyond 80 characters?  Do you think it is a good idea?

I think you are miss-understanding the point of checkpatch here.  It has
flagged this line as suspect.  You are happy it is better as it is.  You
should therefore leave it as it is as you "are happy to justify that
checkpatch failure".  Checkpatch is a style guide, if it complains you
should think about its complaint and act as you see fit in response.
Sometimes you will ignore it, that is fine.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Andy Whitcroft
On Sun, Dec 02, 2007 at 01:03:35PM +0100, Christer Weinigel wrote:
 Hi Andy,
 
 you seem to be the last person messing around with checkpatch.pl so I'm
 addressing this to you. :-)
 
 checkpatch complains about the following:
 
 WARNING: no space between function name and open parenthesis '('
 #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
 +   list_for_each_entry (transfer, message-transfers, transfer_list) {
 
 which I think is a bit bogus since it actually is a for statement in
 disguise.  The following patch adds list_for_each to the list of things
 that look like functions that it shouldn't complain about.

We have had some stabs at changing this, but no consensus was reached on
whether it was a for or a function.  My memory is of there being
slightly more without a space tenders than the other and so it has not
been changed.  This thread also seems so far to have not really
generated a concensus.  So I would tend to leave things as they are.  

A third option might be to accept either on *for_each* constructs.
That might tend to lead to divergance.  Difficult.  However, also see my
later comments on style guide.

 
 By the way, what is the consensus on lines over 80 characters?
 checkpatch complains about the following:
 
 WARNING: line over 80 characters
 #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
 +   printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
 Technologies AB\n);
 
 I can of course break this into:
 
 printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
  Technologies AB\n);
 
 but in my opinion that becomes more even unreadable.  Would it be
 possible to add a special case so that checkpatch ignores long strings
 that go beyond 80 characters?  Do you think it is a good idea?

I think you are miss-understanding the point of checkpatch here.  It has
flagged this line as suspect.  You are happy it is better as it is.  You
should therefore leave it as it is as you are happy to justify that
checkpatch failure.  Checkpatch is a style guide, if it complains you
should think about its complaint and act as you see fit in response.
Sometimes you will ignore it, that is fine.

-apw
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread pHilipp Zabel
On Dec 2, 2007 1:03 PM, Christer Weinigel [EMAIL PROTECTED] wrote:
 Hi Andy,

 you seem to be the last person messing around with checkpatch.pl so I'm
 addressing this to you. :-)

 checkpatch complains about the following:

 WARNING: no space between function name and open parenthesis '('
 #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
 +   list_for_each_entry (transfer, message-transfers, transfer_list) {

 which I think is a bit bogus since it actually is a for statement in
 disguise.  The following patch adds list_for_each to the list of things
 that look like functions that it shouldn't complain about.

 By the way, what is the consensus on lines over 80 characters?
 checkpatch complains about the following:

 WARNING: line over 80 characters
 #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
 +   printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
 Technologies AB\n);

 I can of course break this into:

 printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
Technologies AB\n);

I'd not split it in the middle of a name or phrase if possible.
printk(KERN_INFO S3C24xx SPI DMA driver
(c) 2007 Nordnav Technologies AB\n);
but ...

 but in my opinion that becomes more even unreadable.  Would it be
 possible to add a special case so that checkpatch ignores long strings
 that go beyond 80 characters?  Do you think it is a good idea?

... in this case
   pr_info(S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies AB\n);
might just push it under the limit.

cheers
Philipp
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Christoph Hellwig
On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
 We have had some stabs at changing this, but no consensus was reached on
 whether it was a for or a function.  My memory is of there being
 slightly more without a space tenders than the other and so it has not
 been changed.  This thread also seems so far to have not really
 generated a concensus.  So I would tend to leave things as they are.  
 
 A third option might be to accept either on *for_each* constructs.
 That might tend to lead to divergance.  Difficult.  However, also see my
 later comments on style guide.

Pretty much all core code uses list_for_each_entry( so new code should
follow that example.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Arnaldo Carvalho de Melo
Em Thu, Jan 03, 2008 at 12:26:10PM +, Christoph Hellwig escreveu:
 On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
  We have had some stabs at changing this, but no consensus was reached on
  whether it was a for or a function.  My memory is of there being
  slightly more without a space tenders than the other and so it has not
  been changed.  This thread also seems so far to have not really
  generated a concensus.  So I would tend to leave things as they are.  
  
  A third option might be to accept either on *for_each* constructs.
  That might tend to lead to divergance.  Difficult.  However, also see my
  later comments on style guide.
 
 Pretty much all core code uses list_for_each_entry( so new code should
 follow that example.

Agreed, CodingStyle is not about mindless consistency such as for ( is
the right thing, so list_for_each ( is consistent with it, it is about
codifying practice contributors got used to over the years.

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Tomas Carnecky

Christer Weinigel wrote:

By the way, what is the consensus on lines over 80 characters?
checkpatch complains about the following:

WARNING: line over 80 characters
#762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
+   printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies 
AB\n);

I can of course break this into:

printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
   Technologies AB\n);

but in my opinion that becomes more even unreadable.  Would it be
possible to add a special case so that checkpatch ignores long strings
that go beyond 80 characters?  Do you think it is a good idea?


At the top of the file add a #define and use that in the code? Some 
drivers define their version/author etc that way and then just

printk(DRIVER_VERSION DRIVER_AUTHOR);

tom
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Benny Halevy
On Jan. 03, 2008, 14:30 +0200, Arnaldo Carvalho de Melo [EMAIL PROTECTED] 
wrote:
 Em Thu, Jan 03, 2008 at 12:26:10PM +, Christoph Hellwig escreveu:
 On Thu, Jan 03, 2008 at 11:10:58AM +, Andy Whitcroft wrote:
 We have had some stabs at changing this, but no consensus was reached on
 whether it was a for or a function.  My memory is of there being
 slightly more without a space tenders than the other and so it has not
 been changed.  This thread also seems so far to have not really
 generated a concensus.  So I would tend to leave things as they are.  

 A third option might be to accept either on *for_each* constructs.
 That might tend to lead to divergance.  Difficult.  However, also see my
 later comments on style guide.
 Pretty much all core code uses list_for_each_entry( so new code should
 follow that example.
 
 Agreed, CodingStyle is not about mindless consistency such as for ( is
 the right thing, so list_for_each ( is consistent with it, it is about
 codifying practice contributors got used to over the years.
 

Why mindless?
Coding style is also about giving the coding language logic a graphical
representation.  Following a convention that flow control keywords
such as if, for, or while are distinguished from function calls
by use of a space after the keyword really helps the code readability
regardless of how people used to code it in the past...
The for_each_* macros are clearly not function calls but rather translate
to for () flow control constructs hence they should follow the same convention.
FWIW, I think that changing the existing convention is worth it in this case.

Benny

 - Arnaldo

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2008-01-03 Thread Christer Weinigel
On Thu, 03 Jan 2008 13:34:45 +0100
Tomas Carnecky [EMAIL PROTECTED] wrote:

 Christer Weinigel wrote:
  By the way, what is the consensus on lines over 80 characters?
  checkpatch complains about the following:
  
  WARNING: line over 80 characters
  #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720:
  +   printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav
  Technologies AB\n);
  
  I can of course break this into:
  
  printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav 
 Technologies AB\n);
  
  but in my opinion that becomes more even unreadable.  Would it be
  possible to add a special case so that checkpatch ignores long
  strings that go beyond 80 characters?  Do you think it is a good
  idea?
 
 At the top of the file add a #define and use that in the code? Some 
 drivers define their version/author etc that way and then just
 printk(DRIVER_VERSION DRIVER_AUTHOR);

That only solves this specific problem.  For debugging printks, which
often become quite wide, it would make the code even more unreadable to
add lots of defines just to keep things within 80 cols.

  /Christer
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2007-12-02 Thread Valdis . Kletnieks
On Sun, 02 Dec 2007 13:03:35 +0100, Christer Weinigel said:

> WARNING: no space between function name and open parenthesis '('
> #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
> +   list_for_each_entry (transfer, >transfers, transfer_list) {
> 
> which I think is a bit bogus since it actually is a for statement in
> disguise.

That's how it's *implemented*.  I obviously studied too much Lisp as an
undergrad, I keep thinking of list_for_* helpers as functions that take a
lambda-expression as input. In which case, it's a function and no blank is used.

:)


pgp0p0nA2PLDM.pgp
Description: PGP signature


Re: [PATCH] teach checkpatch.pl about list_for_each

2007-12-02 Thread Arnaldo Carvalho de Melo
Em Sun, Dec 02, 2007 at 01:03:35PM +0100, Christer Weinigel escreveu:
> Hi Andy,
> 
> you seem to be the last person messing around with checkpatch.pl so I'm
> addressing this to you. :-)
> 
> checkpatch complains about the following:
> 
> WARNING: no space between function name and open parenthesis '('
> #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
> +   list_for_each_entry (transfer, >transfers, transfer_list) {
> 
> which I think is a bit bogus since it actually is a for statement in
> disguise.  The following patch adds list_for_each to the list of things
> that look like functions that it shouldn't complain about.

Then you would have to do this for tons other *_for_each*, such as
hlist_for_each, etc, but:

[EMAIL PROTECTED] net-2.6.25]$ find . -name "*.[ch]" | xargs grep
'_for_each[a-z_]*(' | wc -l
4370
[EMAIL PROTECTED] net-2.6.25]$ find . -name "*.[ch]" | xargs grep
'_for_each[a-z_]* (' | wc -l
160
[EMAIL PROTECTED] net-2.6.25]$

I'd say that the common practice in the * _for_each_* use is to do just
what checkpatch does right now, complain if the is a space. Ah, and that
is also my personal preference 8-)

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2007-12-02 Thread Arnaldo Carvalho de Melo
Em Sun, Dec 02, 2007 at 01:03:35PM +0100, Christer Weinigel escreveu:
 Hi Andy,
 
 you seem to be the last person messing around with checkpatch.pl so I'm
 addressing this to you. :-)
 
 checkpatch complains about the following:
 
 WARNING: no space between function name and open parenthesis '('
 #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
 +   list_for_each_entry (transfer, message-transfers, transfer_list) {
 
 which I think is a bit bogus since it actually is a for statement in
 disguise.  The following patch adds list_for_each to the list of things
 that look like functions that it shouldn't complain about.

Then you would have to do this for tons other *_for_each*, such as
hlist_for_each, etc, but:

[EMAIL PROTECTED] net-2.6.25]$ find . -name *.[ch] | xargs grep
'_for_each[a-z_]*(' | wc -l
4370
[EMAIL PROTECTED] net-2.6.25]$ find . -name *.[ch] | xargs grep
'_for_each[a-z_]* (' | wc -l
160
[EMAIL PROTECTED] net-2.6.25]$

I'd say that the common practice in the * _for_each_* use is to do just
what checkpatch does right now, complain if the is a space. Ah, and that
is also my personal preference 8-)

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] teach checkpatch.pl about list_for_each

2007-12-02 Thread Valdis . Kletnieks
On Sun, 02 Dec 2007 13:03:35 +0100, Christer Weinigel said:

 WARNING: no space between function name and open parenthesis '('
 #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478:
 +   list_for_each_entry (transfer, message-transfers, transfer_list) {
 
 which I think is a bit bogus since it actually is a for statement in
 disguise.

That's how it's *implemented*.  I obviously studied too much Lisp as an
undergrad, I keep thinking of list_for_* helpers as functions that take a
lambda-expression as input. In which case, it's a function and no blank is used.

:)


pgp0p0nA2PLDM.pgp
Description: PGP signature