Outstanding patches for errors picked up via sparse
Subsystems (2)
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] sizedrivers/scsi/pmcraid.c:4100:23: got restricted gfp_tdrivers/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] flagsdrivers/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 |_______/
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/6885298minor firmware version printing problem:
drivers/scsi/megaraid.c:313:65: warning: right shift by bigger than source valuedrivers/scsi/megaraid.c:315:65: warning: right shift by bigger than source valuedrivers/scsi/megaraid.c:319:67: warning: right shift by bigger than source valuedrivers/scsi/megaraid.c:321:67: warning: right shift by bigger than source valueadapter->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.hhas: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..."
--- 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,