aacraid: use lower snprintf() limit

PATCH Superseded Dan Carpenter 3 participants 7 messages Oct 5, 2011 – Oct 11, 2011

Subsystems (3)

SCSI Kernel janitors Aacraid SCSI RAID driver

lore

Share & tools

lore

Dan Carpenter

author

2011-10-05 05:50 UTC #0 lore raw

PATCH 1/1 aacraid: use lower snprintf() limit

This is just a cleanup, to silence static checker warnings. It doesn't change how the code works.

buf[] can either be BUF_SIZE if this is called from sysfs, or it can be 16 if it's called from aac_get_adapter_info() via aac_get_serial_number(). We use the smaller limit here.

sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo) is 12 so there is actually no chance of hitting either limit.

Signed-off-by Dan Carpenter <[email protected]>

drivers/scsi/aacraid/linit.c +7 -2

index 3382475..ea8d96e 100644
@@ -894,15 +894,20 @@ static ssize_t aac_show_serial_number(struct device *device,
 	int len = 0;
 
 	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)
-		len = snprintf(buf, PAGE_SIZE, "%06X\n",
+		len = snprintf(buf, 16, "%06X\n",
 		  le32_to_cpu(dev->adapter_info.serial[0]));
 	if (len &&
 	  !memcmp(&dev->supplement_adapter_info.MfgPcbaSerialNo[
 	    sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo)-len],
 	  buf, len-1))
-		len = snprintf(buf, PAGE_SIZE, "%.*s\n",
+		len = snprintf(buf, 16, "%.*s\n",
 		  (int)sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo),
 		  dev->supplement_adapter_info.MfgPcbaSerialNo);
+
+
+	if (len > 16)
+		len = 16;
+
 	return len;
 }

Rolf Eike Beer

2011-10-05 07:35 UTC #1 lore raw

Quoted 24 lines

This is just a cleanup, to silence static checker warnings. It doesn't change how the code works.
buf[] can either be BUF_SIZE if this is called from sysfs, or it can be 16 if it's called from aac_get_adapter_info() via aac_get_serial_number(). We use the smaller limit here.
sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo) is 12 so there is actually no chance of hitting either limit.
Signed-off-by Dan Carpenter <[email protected]>
drivers/scsi/aacraid/linit.c +1 -1
index 3382475..ea8d96e 100644
@@ -894,15 +894,20 @@ static ssize_t aac_show_serial_number(struct device
 *device,
 	int len = 0;
 
 	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)
-		len = snprintf(buf, PAGE_SIZE, "%06X\n",
+		len = snprintf(buf, 16, "%06X\n",
 		  le32_to_cpu(dev->adapter_info.serial[0]));

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device · line 908

Quoted 10 lines

drivers/scsi/aacraid/linit.c +3 -1

 	if (len &&
 	  !memcmp(&dev->supplement_adapter_info.MfgPcbaSerialNo[
 	    sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo)-len],
 	  buf, len-1))
-		len = snprintf(buf, PAGE_SIZE, "%.*s\n",
+		len = snprintf(buf, 16, "%.*s\n",
 		  (int)sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo),
 		  dev->supplement_adapter_info.MfgPcbaSerialNo);
+
+

One newline too much.

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909

Quoted 2 lines

drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;

max()?

Eike

Achim Leubner

Acked-by

2011-10-05 12:19 UTC #2 lore raw

Looks good so far. But do we really need the last two lines?

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909

Quoted 2 lines

drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;

Acked-by Achim Leubner <[email protected]>

Thanks,
Achim

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Rolf Eike Beer
Sent: Mittwoch, 5. Oktober 2011 09:35
To: Dan Carpenter
Cc: Adaptec OEM Raid Solutions; James E.J. Bottomley; [email protected]; [email protected]; [email protected]
Subject: Re: [patch] [SCSI] aacraid: use lower snprintf() limit

Quoted 7 lines

This is just a cleanup, to silence static checker warnings. It doesn't change how the code works.
buf[] can either be BUF_SIZE if this is called from sysfs, or it can be 16 if it's called from aac_get_adapter_info() via aac_get_serial_number(). We use the smaller limit here.

Review

Quoted 6 lines

sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo) is 12 so there is actually no chance of hitting either limit.
Signed-off-by Dan Carpenter <[email protected]>
(quoted diff) +0 -0

b/drivers/scsi/aacraid/linit.c

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct

Quoted 4 lines

index 3382475..ea8d96e 100644
drivers/scsi/aacraid/linit.c +0 -0
@@ -894,15 +894,20 @@ static ssize_t aac_show_serial_number(struct

device

Quoted 4 lines

drivers/scsi/aacraid/linit.c +0 -0

 *device,
 	int len = 0;
 
 	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 913

Quoted 10 lines

drivers/scsi/aacraid/linit.c +2 -2

-		len = snprintf(buf, PAGE_SIZE, "%06X\n",
+		len = snprintf(buf, 16, "%06X\n",
 		  le32_to_cpu(dev->adapter_info.serial[0]));
 	if (len &&
 	  !memcmp(&dev->supplement_adapter_info.MfgPcbaSerialNo[
 	    sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo)-len],
 	  buf, len-1))
-		len = snprintf(buf, PAGE_SIZE, "%.*s\n",
+		len = snprintf(buf, 16, "%.*s\n",

(int)sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo),

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 907

Quoted 3 lines

drivers/scsi/aacraid/linit.c +2 -0

 		  dev->supplement_adapter_info.MfgPcbaSerialNo);
+
+

One newline too much.

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909

Quoted 2 lines

drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;

max()?

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

Dan Carpenter

author

2011-10-06 14:30 UTC #3 lore raw

Quoted 4 lines · Achim Leubner

Looks good so far. But do we really need the last two lines?
drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909
Quoted 2 lines
drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;

snprintf() returns the number of characters that it _would_ have copied if it had space. We want to return the number of bytes actually copied.

Again, the snprintf() values do fit so this change doesn't actually make any difference.

regards,
dan carpenter

Achim Leubner

2011-10-11 14:46 UTC #4 lore raw

Agreed.

Thanks,
Achim

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Donnerstag, 6. Oktober 2011 16:31
To: Achim Leubner
Cc: Rolf Eike Beer; Adaptec OEM Raid Solutions; James E.J. Bottomley; [email protected]; [email protected]; [email protected]
Subject: Re: [patch] [SCSI] aacraid: use lower snprintf() limit

Quoted 4 lines · Achim Leubner

Looks good so far. But do we really need the last two lines?
drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909
Quoted 2 lines
drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;

snprintf() returns the number of characters that it _would_ have copied if it had space. We want to return the number of bytes actually copied.

Again, the snprintf() values do fit so this change doesn't actually make any difference.

regards,
dan carpenter

Dan Carpenter

author

2011-10-06 14:27 UTC #5 lore raw

In reply to Rolf Eike Beer

Quoted 6 lines · Rolf Eike Beer

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 907
Quoted 3 lines
drivers/scsi/aacraid/linit.c +2 -0
 		  dev->supplement_adapter_info.MfgPcbaSerialNo);
+
+
One newline too much.

True. I wish checkpatch.pl checked for that...

Quoted 4 lines

drivers/scsi/aacraid/linit.c static ssize_t aac_show_serial_number(struct device *device, · line 909
Quoted 2 lines
drivers/scsi/aacraid/linit.c +2 -0
+	if (len > 16)
+		len = 16;
max()?

min() actually. I don't know that it improves the readability, but can do.

regards,
dan carpenter

Dan Carpenter

author

2011-10-08 10:45 UTC #6 lore raw

PATCH v2 1/1 aacraid: use lower snprintf() limit

In reply to Rolf Eike Beer

This is just a cleanup, to silence static checker warnings. It doesn't change how the code works.

buf[] can either be BUF_SIZE if this is called from sysfs, or it can be 16 if it's called from aac_get_adapter_info() via aac_get_serial_number(). We use the smaller limit here.

sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo) is 12 so there is actually no chance of hitting either limit.

Signed-off-by Dan Carpenter <[email protected]>

---
v2: fix a whitespace issue, and use the min() macro.

drivers/scsi/aacraid/linit.c +4 -3

index 3382475..4aa76d6 100644
@@ -894,16 +894,17 @@ static ssize_t aac_show_serial_number(struct device *device,
 	int len = 0;
 
 	if (le32_to_cpu(dev->adapter_info.serial[0]) != 0xBAD0)
-		len = snprintf(buf, PAGE_SIZE, "%06X\n",
+		len = snprintf(buf, 16, "%06X\n",
 		  le32_to_cpu(dev->adapter_info.serial[0]));
 	if (len &&
 	  !memcmp(&dev->supplement_adapter_info.MfgPcbaSerialNo[
 	    sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo)-len],
 	  buf, len-1))
-		len = snprintf(buf, PAGE_SIZE, "%.*s\n",
+		len = snprintf(buf, 16, "%.*s\n",
 		  (int)sizeof(dev->supplement_adapter_info.MfgPcbaSerialNo),
 		  dev->supplement_adapter_info.MfgPcbaSerialNo);
-	return len;
+
+	return min(len, 16);
 }
 
 static ssize_t aac_show_max_channel(struct device *device,