[patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()

[patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()

[Dan Carpenter]

The "fmt" variable might be used uninitialized, it should be set to NULL
at the start.

Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/usb/storage/debug.c b/drivers/usb/storage/debug.c
index 57bf3ad..fd00509 100644
--- a/drivers/usb/storage/debug.c
+++ b/drivers/usb/storage/debug.c
@@ -164,7 +164,8 @@ void usb_stor_show_sense(const struct us_data *us,
 			 unsigned char asc,
 			 unsigned char ascq)
 {
-	const char *what, *keystr, *fmt;
+	const char *fmt = NULL;
+	const char *what, *keystr;
 
 	keystr = scsi_sense_key_string(key);
 	what = scsi_extd_sense_format(asc, ascq, &fmt);

Re: [patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()

[James Bottomley]

On Sat, 2014-11-29 at 15:48 +0300, Dan Carpenter wrote:
> The "fmt" variable might be used uninitialized, it should be set to NULL
> at the start.
> 
> Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Hm, that' falls into the category of an interface that's hard to get
right if every caller has to remember to set fmt to NULL.

Could you instead set *fmt to NULL in scsi_extd_sense_format()?  That
way the interface is much easier.

Thanks,

James

Re: [patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()

[Dan Carpenter]

On Sat, Nov 29, 2014 at 08:07:55AM -0800, James Bottomley wrote:
> On Sat, 2014-11-29 at 15:48 +0300, Dan Carpenter wrote:
> > The "fmt" variable might be used uninitialized, it should be set to NULL
> > at the start.
> > 
> > Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Hm, that' falls into the category of an interface that's hard to get
> right if every caller has to remember to set fmt to NULL.
> 
> Could you instead set *fmt to NULL in scsi_extd_sense_format()?  That
> way the interface is much easier.
> 

Sure.  I will send that tomorrow.

regards,
dan carpenter

[patch v2] scsi: set fmt to NULL scsi_extd_sense_format() by default

[Dan Carpenter]

One of the two callers passes an unintialized pointer as "fmt" and
expects it to be set to NULL if there is no format string.  Let's make
this function work as expected.

Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  Change scsi_extd_sense_format() instead of the caller

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index aee62f6..e2068a2 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1271,6 +1271,7 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
 	int i;
 	unsigned short code = ((asc << 8) | ascq);
 
+	*fmt = NULL;
 	for (i = 0; additional[i].text; i++)
 		if (additional[i].code12 == code)
 			return additional[i].text;
@@ -1282,6 +1283,8 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
 			return additional2[i].str;
 		}
 	}
+#else
+	*fmt = NULL;
 #endif
 	return NULL;
 }

Re: [patch v2] scsi: set fmt to NULL scsi_extd_sense_format() by default

[James Bottomley]

On Tue, 2014-12-02 at 14:07 +0300, Dan Carpenter wrote:
> One of the two callers passes an unintialized pointer as "fmt" and
> expects it to be set to NULL if there is no format string.  Let's make
> this function work as expected.
> 
> Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  Change scsi_extd_sense_format() instead of the caller

Reviewed-by: James Bottomley <JBottomley@Parallels.com>

James