Skip to contentSkip to conversation

Outstanding patches for errors picked up via sparse

DISCUSSION Dr. David Alan Gilbert 2 participants 2 messages Apr 3, 2011 – Apr 4, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi,
I've been trawling through sparse logs for a few months now, and I've noticed
that there are a few fixes for errors that have been out there for a while. This mail summarises those, so anyone else trawling through sparse knows not to bother digging.

Note that these all fix actual screwups as opposed to just removing warnings.

(It would also be nice if it gently pushed them forward into the main kernel)

Dave
--
Fixed by Joe Perches 2010-10-31:

https://lkml.org/lkml/2010/10/31/123 (resent as https://lkml.org/lkml/2010/11/2/278)
Swapped kmalloc parameters:

drivers/scsi/pmcraid.c:4100:23: warning: incorrect type in argument 1 (different base types)
drivers/scsi/pmcraid.c:4100:23: expected unsigned long [unsigned] [usertype] size
drivers/scsi/pmcraid.c:4100:23: got restricted gfp_t
drivers/scsi/pmcraid.c:4100:35: warning: incorrect type in argument 2 (different base types)
drivers/scsi/pmcraid.c:4100:35: expected restricted gfp_t [usertype] flags
drivers/scsi/pmcraid.c:4100:35: got unsigned long

        hdr = kmalloc(GFP_KERNEL, sizeof(struct pmcraid_ioctl_header));

--
Fix by Vasiliy Kulikov 2010-09-12

https://patchwork.kernel.org/patch/173492/
http://liquorix.net/sources/patches/suse/patches.fixes/acpi_ec_sys_access_user_space_with_get_user.patch

missing get_user/put_user

drivers/acpi/ec_sys.c:46:21: warning: cast removes address space of expression drivers/acpi/ec_sys.c:77:21: warning: cast removes address space of expression

--

Fixed by Randy Dunlap 2010-06-07
http://kerneltrap.org/mailarchive/linux-scsi/2010/6/8/6885298

minor firmware version printing problem:

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

                         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);

yet megaraid.h has:

        u8      fw_version[16];         /* printable ASCI string */
        u8      bios_version[16];       /* printable ASCI string */

Although you do have to wonder if the comment is right there then maybe the sprintf is more wrong than the fix fixes.

--
Fix by Russ Gorby 2010-11-24
http://www.mail-archive.com/[email protected]/msg00692.html

drivers/tty/serial/ifx6x60.c:354:31: warning: right shift by bigger than source value drivers/tty/serial/ifx6x60.c:355:39: warning: right shift by bigger than source value
Those are masking an 8 bit byte from a buffer but those are testing bits after 8th bit

Note that Russ's patch is a bit more complex and doesn't just fix that test.

Signature · 4 lines
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\ gro.gilbert @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/
#1 lore raw
Quoted 34 lines · On Mon, 4 Apr 2011 00:19:02 +0100 Dr. David Alan Gilbert wrote:

Hi,
I've been trawling through sparse logs for a few months now, and I've noticed
that there are a few fixes for errors that have been out there for a while. This mail summarises those, so anyone else trawling through sparse knows not to bother digging.

Note that these all fix actual screwups as opposed to just removing warnings.

(It would also be nice if it gently pushed them forward into the main kernel)

Dave
--

Fixed by Randy Dunlap 2010-06-07
http://kerneltrap.org/mailarchive/linux-scsi/2010/6/8/6885298

minor firmware version printing problem:

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

                         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);
Quoted 8 lines

yet megaraid.h has:

        u8      fw_version[16];         /* printable ASCI string */
        u8      bios_version[16];       /* printable ASCI string */

Although you do have to wonder if the comment is right there then maybe the sprintf is more wrong than the fix fixes.

--

Here is an updated version of that patch:

---

From: Randy Dunlap <[email protected]>

Fix sparse warnings of right shift bigger than source value size:

drivers/scsi/megaraid.c:311:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:313:65: warning: right shift by bigger than source value drivers/scsi/megaraid.c:317:67: warning: right shift by bigger than source value drivers/scsi/megaraid.c:319:67: warning: right shift by bigger than source value

Patch suggestion from email by Al Viro:

"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..."

Cc Al Viro <[email protected]>
Cc Neela Syam Kolli <[email protected]>
---
 drivers/scsi/megaraid.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/scsi/megaraid.c +4 -4
@@ -308,15 +308,15 @@ mega_query_adapter(adapter_t *adapter)
 	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] >> 4,
 			 adapter->product_info.fw_version[1] & 0x0f,
-			 adapter->product_info.fw_version[0] >> 8,
+			 adapter->product_info.fw_version[0] >> 4,
 			 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] >> 4,
 			 adapter->product_info.bios_version[1] & 0x0f,
-			 adapter->product_info.bios_version[0] >> 8,
+			 adapter->product_info.bios_version[0] >> 4,
 			 adapter->product_info.bios_version[0] & 0x0f);
 	} else {
 		memcpy(adapter->fw_version,