LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Checkpatch.pl failure
@ 2008-01-14 15:48 James Bottomley
  2008-01-14 16:18 ` Benny Halevy
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: James Bottomley @ 2008-01-14 15:48 UTC (permalink / raw)
  To: rdunlap, apw, jschopp; +Cc: linux-kernel

This error:

ERROR: no space before that close parenthesis ')'
#501: FILE: drivers/scsi/dpt_i2o.c:2299:
+               if (dev_status == 0x02 /*CHECK_CONDITION*/) {

Is definitely wrong.  I think it's stripped the comments so now the if
looks to have a space before the bracket, but stylistically the
complaint it has errored out for is wrong.

James



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Checkpatch.pl failure
  2008-01-14 15:48 Checkpatch.pl failure James Bottomley
@ 2008-01-14 16:18 ` Benny Halevy
  2008-01-14 17:10 ` Andy Whitcroft
  2008-01-14 19:04 ` Salyzyn, Mark
  2 siblings, 0 replies; 4+ messages in thread
From: Benny Halevy @ 2008-01-14 16:18 UTC (permalink / raw)
  To: James Bottomley; +Cc: rdunlap, apw, jschopp, linux-kernel

On Jan. 14, 2008, 17:48 +0200, James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> This error:
> 
> ERROR: no space before that close parenthesis ')'
> #501: FILE: drivers/scsi/dpt_i2o.c:2299:
> +               if (dev_status == 0x02 /*CHECK_CONDITION*/) {
> 
> Is definitely wrong.  I think it's stripped the comments so now the if
> looks to have a space before the bracket, but stylistically the
> complaint it has errored out for is wrong.

I've seen similar complaints as well and I agree with James that
they seem bogus.  I think that the comment should be treated as
part of the grammar and not just stripped out and then you can
even add checks about allowed spacing before and after the comment.

> 
> James

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Checkpatch.pl failure
  2008-01-14 15:48 Checkpatch.pl failure James Bottomley
  2008-01-14 16:18 ` Benny Halevy
@ 2008-01-14 17:10 ` Andy Whitcroft
  2008-01-14 19:04 ` Salyzyn, Mark
  2 siblings, 0 replies; 4+ messages in thread
From: Andy Whitcroft @ 2008-01-14 17:10 UTC (permalink / raw)
  To: James Bottomley; +Cc: rdunlap, jschopp, linux-kernel

On Mon, Jan 14, 2008 at 09:48:53AM -0600, James Bottomley wrote:
> This error:
> 
> ERROR: no space before that close parenthesis ')'
> #501: FILE: drivers/scsi/dpt_i2o.c:2299:
> +               if (dev_status == 0x02 /*CHECK_CONDITION*/) {
> 
> Is definitely wrong.  I think it's stripped the comments so now the if
> looks to have a space before the bracket, but stylistically the
> complaint it has errored out for is wrong.

Yes, that is kinda wrong.  Its a difficult one to deal with nicely as
basically spacing goes to hell when comments are wedged in the middle.
The rules basically go out the window.  In the next version I do at
least have a handle on where comments are, but we have not yet
progressed to where we can simply get the spacing checks right.

I'll think more on it, and see what we can do.

-apw

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: Checkpatch.pl failure
  2008-01-14 15:48 Checkpatch.pl failure James Bottomley
  2008-01-14 16:18 ` Benny Halevy
  2008-01-14 17:10 ` Andy Whitcroft
@ 2008-01-14 19:04 ` Salyzyn, Mark
  2 siblings, 0 replies; 4+ messages in thread
From: Salyzyn, Mark @ 2008-01-14 19:04 UTC (permalink / raw)
  To: 'linux-scsi@vger.kernel.org'
  Cc: 'linux-kernel', 'James Bottomley',
	'jschopp@austin.ibm.com', 'apw@shadowen.org',
	'rdunlap@xenotime.net', 'FUJITA Tomonori',
	'fujita.tomonori@lab.ntt.co.jp'

[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]

Suppress one of the bogus checkpatch.pl error, the side-effect of the error highlighted that this constant should be replaced by an existing manifest. checkpatch.pl needs to be corrected to accept the comment style to deal with the other cases should they ever be touched by future patches. This is a tangled set of coat hangers, tug on one and never know how complicated of a mess might follow!

This attached patch is against current scsi-misc-2.6.

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's handling of patch attachments, use the attachment, not the inline patch.

Signed-off-by: Mark Salyzyn <aacraid@adaptec.com>

 drivers/scsi/dpt_i2o.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -ru a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
--- a/drivers/scsi/dpt_i2o.c    2008-01-14 13:39:11.086600955 -0500
+++ b/drivers/scsi/dpt_i2o.c    2008-01-14 13:41:44.813246497 -0500
@@ -2296,7 +2296,7 @@

                // copy over the request sense data if it was a check
                // condition status
-               if (dev_status == 0x02 /*CHECK_CONDITION*/) {
+               if (dev_status == SAM_STAT_CHECK_CONDITION) {
                        u32 len = min(SCSI_SENSE_BUFFERSIZE, 40);
                        // Copy over the sense data
                        memcpy_fromio(cmd->sense_buffer, (reply+28) , len);

This geek joke brought to you by -- Mark Salyzyn

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of
> James Bottomley
> Sent: Monday, January 14, 2008 10:49 AM
> To: rdunlap@xenotime.net; apw@shadowen.org; jschopp@austin.ibm.com
> Cc: linux-kernel
> Subject: Checkpatch.pl failure
>
> This error:
>
> ERROR: no space before that close parenthesis ')'
> #501: FILE: drivers/scsi/dpt_i2o.c:2299:
> +               if (dev_status == 0x02 /*CHECK_CONDITION*/) {
>
> Is definitely wrong.  I think it's stripped the comments so now the if
> looks to have a space before the bracket, but stylistically the
> complaint it has errored out for is wrong.
>
> James

[-- Attachment #2: dpt_i2o_CHECK_CONDITION.patch --]
[-- Type: application/octet-stream, Size: 525 bytes --]

diff -ru a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
--- a/drivers/scsi/dpt_i2o.c	2008-01-14 13:39:11.086600955 -0500
+++ b/drivers/scsi/dpt_i2o.c	2008-01-14 13:41:44.813246497 -0500
@@ -2296,7 +2296,7 @@
 
 		// copy over the request sense data if it was a check
 		// condition status
-		if (dev_status == 0x02 /*CHECK_CONDITION*/) {
+		if (dev_status == SAM_STAT_CHECK_CONDITION) {
 			u32 len = min(SCSI_SENSE_BUFFERSIZE, 40);
 			// Copy over the sense data
 			memcpy_fromio(cmd->sense_buffer, (reply+28) , len);

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-01-14 19:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-14 15:48 Checkpatch.pl failure James Bottomley
2008-01-14 16:18 ` Benny Halevy
2008-01-14 17:10 ` Andy Whitcroft
2008-01-14 19:04 ` Salyzyn, Mark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).