[patch] [SCSI] aacraid: use lower snprintf() limit

[patch] [SCSI] aacraid: use lower snprintf() limit

[Dan Carpenter]

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 <dan.carpenter@oracle.com>

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 3382475..ea8d96e 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -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;
 }
 

Re: [patch] [SCSI] aacraid: use lower snprintf() limit

[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 <dan.carpenter@oracle.com>
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 3382475..ea8d96e 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -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);
> +
> +

One newline too much.

> +	if (len > 16)
> +		len = 16;

max()?

Eike

RE: [patch] [SCSI] aacraid: use lower snprintf() limit

[Achim Leubner]

Looks good so far. But do we really need the last two lines?
> +	if (len > 16)
> +		len = 16;

Acked-by: Achim Leubner <Achim_Leubner@pmc-sierra.com> 

Thanks,
Achim


-----Original Message-----
From: linux-scsi-owner@vger.kernel.org
[mailto:linux-scsi-owner@vger.kernel.org] 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;
linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
kernel-janitors@vger.kernel.org
Subject: Re: [patch] [SCSI] 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 <dan.carpenter@oracle.com>
>
> diff --git a/drivers/scsi/aacraid/linit.c
b/drivers/scsi/aacraid/linit.c
> index 3382475..ea8d96e 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -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);
> +
> +

One newline too much.

> +	if (len > 16)
> +		len = 16;

max()?

Eike

Re: [patch] [SCSI] aacraid: use lower snprintf() limit

[Dan Carpenter]

On Wed, Oct 05, 2011 at 09:35:13AM +0200, Rolf Eike Beer wrote:
> >  		  dev->supplement_adapter_info.MfgPcbaSerialNo);
> > +
> > +
> 
> One newline too much.
> 

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

> > +	if (len > 16)
> > +		len = 16;
> 
> max()?

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

regards,
dan carpenter

Re: [patch] [SCSI] aacraid: use lower snprintf() limit

[Dan Carpenter]

On Wed, Oct 05, 2011 at 05:19:12AM -0700, Achim Leubner wrote:
> Looks good so far. But do we really need the last two lines?
> > +	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

[patch v2] [SCSI] aacraid: use lower snprintf() limit

[Dan Carpenter]

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 <dan.carpenter@oracle.com>
---
v2:  fix a whitespace issue, and use the min() macro.

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 3382475..4aa76d6 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -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,

RE: [patch] [SCSI] aacraid: use lower snprintf() limit

[Achim Leubner]

Agreed.

Thanks,
Achim

-----Original Message-----
From: Dan Carpenter [mailto:dan.carpenter@oracle.com] 
Sent: Donnerstag, 6. Oktober 2011 16:31
To: Achim Leubner
Cc: Rolf Eike Beer; Adaptec OEM Raid Solutions; James E.J. Bottomley;
linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
kernel-janitors@vger.kernel.org
Subject: Re: [patch] [SCSI] aacraid: use lower snprintf() limit

On Wed, Oct 05, 2011 at 05:19:12AM -0700, Achim Leubner wrote:
> Looks good so far. But do we really need the last two lines?
> > +	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