megaraid sparse warnings

DISCUSSION Randy Dunlap 2 participants 4 messages Feb 14, 2010 – May 26, 2010 SCSI

lore

Share & tools

lore

Randy Dunlap

author

2010-02-14 05:23 UTC #0 lore raw

Hi,

sparse produces these warnings:

drivers/scsi/megaraid.c:307:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:309:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:313:67: warning: right shift by bigger than source value drivers/scsi/megaraid.c:315:67: warning: right shift by bigger than source value

on this source code:

	/* use HP firmware and bios version encoding */
	if (adapter->product_info.subsysvid == HP_SUBSYS_VID) {
		sprintf (adapter->fw_version, "%c%d%d.%d%d",
			 adapter->product_info.fw_version[2],
			 adapter->product_info.fw_version[1] >> 8,
			 adapter->product_info.fw_version[1] & 0x0f,
			 adapter->product_info.fw_version[0] >> 8,
			 adapter->product_info.fw_version[0] & 0x0f);
		sprintf (adapter->bios_version, "%c%d%d.%d%d",
			 adapter->product_info.bios_version[2],
			 adapter->product_info.bios_version[1] >> 8,
			 adapter->product_info.bios_version[1] & 0x0f,
			 adapter->product_info.bios_version[0] >> 8,
			 adapter->product_info.bios_version[0] & 0x0f);
	} else {

Are the fw_version and bios_version fields more than 8 bits each for HP subsys VID products? Do HP VID products need a different struct so that they print this info correctly?

thanks,

Signature · 1 line

~Randy

Randy Dunlap

author

2010-05-26 21:28 UTC #1 lore raw

ping??

Quoted 28 lines · On Sat, 13 Feb 2010 21:23:04 -0800 Randy Dunlap wrote:

Hi,
sparse produces these warnings:
drivers/scsi/megaraid.c:307:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:309:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:313:67: warning: right shift by bigger than source value drivers/scsi/megaraid.c:315:67: warning: right shift by bigger than source value
on this source code:
	/* use HP firmware and bios version encoding */
	if (adapter->product_info.subsysvid == HP_SUBSYS_VID) {
		sprintf (adapter->fw_version, "%c%d%d.%d%d",
			 adapter->product_info.fw_version[2],
			 adapter->product_info.fw_version[1] >> 8,
			 adapter->product_info.fw_version[1] & 0x0f,
			 adapter->product_info.fw_version[0] >> 8,
			 adapter->product_info.fw_version[0] & 0x0f);
		sprintf (adapter->bios_version, "%c%d%d.%d%d",
			 adapter->product_info.bios_version[2],
			 adapter->product_info.bios_version[1] >> 8,
			 adapter->product_info.bios_version[1] & 0x0f,
			 adapter->product_info.bios_version[0] >> 8,
			 adapter->product_info.bios_version[0] & 0x0f);
	} else {

Quoted 3 lines

Are the fw_version and bios_version fields more than 8 bits each for HP subsys VID products? Do HP VID products need a different struct so that they print this info correctly?

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Al Viro

2010-05-26 21:39 UTC #2 lore raw

Quoted 18 lines · Randy Dunlap

ping??

Quoted 17 lines

		sprintf (adapter->fw_version, "%c%d%d.%d%d",
			 adapter->product_info.fw_version[2],
			 adapter->product_info.fw_version[1] >> 8,
			 adapter->product_info.fw_version[1] & 0x0f,
			 adapter->product_info.fw_version[0] >> 8,
			 adapter->product_info.fw_version[0] & 0x0f);
		sprintf (adapter->bios_version, "%c%d%d.%d%d",
			 adapter->product_info.bios_version[2],
			 adapter->product_info.bios_version[1] >> 8,
			 adapter->product_info.bios_version[1] & 0x0f,
			 adapter->product_info.bios_version[0] >> 8,
			 adapter->product_info.bios_version[0] & 0x0f);
	} else {

Are the fw_version and bios_version fields more than 8 bits each for HP subsys VID products? Do HP VID products need a different struct so that they print this info correctly?

Since both are claimed to be strings, I really suspect that this >> 8 is misspelled >> 4 and they have a character followed by pair of two-digit packed decimals in there...

Randy Dunlap

author

2010-05-26 22:00 UTC #3 lore raw

Quoted 16 lines · On Wed, 26 May 2010 22:39:47 +0100 Al Viro wrote:

Quoted 14 lines · Randy Dunlap

ping??

Quoted 13 lines

		sprintf (adapter->fw_version, "%c%d%d.%d%d",
			 adapter->product_info.fw_version[2],
			 adapter->product_info.fw_version[1] >> 8,
			 adapter->product_info.fw_version[1] & 0x0f,
			 adapter->product_info.fw_version[0] >> 8,
			 adapter->product_info.fw_version[0] & 0x0f);
		sprintf (adapter->bios_version, "%c%d%d.%d%d",
			 adapter->product_info.bios_version[2],
			 adapter->product_info.bios_version[1] >> 8,
			 adapter->product_info.bios_version[1] & 0x0f,
			 adapter->product_info.bios_version[0] >> 8,
			 adapter->product_info.bios_version[0] & 0x0f);
	} else {

Quoted 7 lines

Quoted 3 lines
Quoted 3 lines
Are the fw_version and bios_version fields more than 8 bits each for HP subsys VID products? Do HP VID products need a different struct so that they print this info correctly?
Since both are claimed to be strings, I really suspect that this >> 8 is misspelled >> 4 and they have a character followed by pair of two-digit packed decimals in there...

Duh. Makes perfect sense.

Thanks, Al.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***