Re: [PATCH]drivers/ata/libata-eh.c:1509 unneeded memset()

On 01/11/10 20:41, Marc Bejarano wrote:

and once more in plain text.  (sorry vger)

2010/1/11 Marc Bejarano[off-list ref]:

quoted

oops.  seems that GMANE's reply function only goes to a single "newsgroup".
  original recipients re-added.
marc
----
From: Marc Bejarano<beej<at>  beej.org>
Subject: Re: [PATCH]drivers/ata/libata-eh.c:1509 unneeded memset()
Newsgroups: gmane.linux.scsi
Date: 2010-01-08 22:21:09 GMT (2 days, 20 hours and 40 minutes ago)

René Bolldorf<xsecute<at>  googlemail.com>  writes:

quoted

On 01/08/10 16:28, James Bottomley wrote:

quoted

On Thu, 2010-01-07 at 14:15 -0500, Jeff Garzik wrote:

quoted

On 12/30/2009 05:59 PM, René Bolldorf wrote:

quoted

We don't need this .

quoted

quoted

quoted

quoted

- /* FIXME: is this needed? */
- memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);

I need a little bit more detail than an unqualified statement...  Did
you audit all paths leading to this code point?

quoted

quoted

But one also here:

u8 *sense_buffer = dev->link->ap->sector_buf;
[...]
err_mask = atapi_eh_request_sense(dev, sense_buffer, sense_key);

Which doesn't look OK because it looks like the sector_buf isn't cleared
(and it is reused).

James

Thank's, you're right. I have overseen this, sry for that.

René: perhaps you'd like to submit a patch that substitutes FIXME comment
for one that explains why the memset is needed, crediting James in the
description?  we may as well gain something permanent from this discussion
that you started :)

cheers,
marc

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo<at>  vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

----

I hope that's good enough explained :-).

Best regards
René

--- ./drivers/ata/libata-eh.c   2010-01-07 23:21:23.622464012 +0100
+++ ./drivers/ata/libata-eh.c   2010-01-11 21:11:19.869092897 +0100

@@ -1505,7 +1505,10 @@ static unsigned int atapi_eh_request_sen

         DPRINTK("ATAPI request sense\n");

-       /* FIXME: is this needed? */
+       /* make sure sense_buf is cleared then atapi_eh_request_sense is 

called.
+        * (to make sure nothing get's reused.)
+        * thanks to James Bottomley
+        */
         memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);

         /* initialize sense_buf with the error register,