diff mbox series

csi: myrs: Convert snprintf to scnprintf

Message ID 20fbbc099e132bb4d3c367da18051c7c@208suo.com
State New
Headers show
Series csi: myrs: Convert snprintf to scnprintf | expand

Commit Message

sunran001@208suo.com July 7, 2023, 7:36 a.m. UTC
Coccinelle reports: WARNING: use scnprintf or sprintf

Adding to that, there has also been some slow migration from snprintf to
scnprintf. This article explains the rationale for this change:
https: //lwn.net/Articles/69419/

Signed-off-by: RAN SUN <sunran001@208suo.com>
---
  drivers/scsi/myrs.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Comments

David Laight July 12, 2023, 7:58 a.m. UTC | #1
From: sunran001@208suo.com
> Sent: 07 July 2023 08:37
> 
> Coccinelle reports: WARNING: use scnprintf or sprintf
> 
> Adding to that, there has also been some slow migration from snprintf to
> scnprintf. This article explains the rationale for this change:
> https: //lwn.net/Articles/69419/
> 
> Signed-off-by: RAN SUN <sunran001@208suo.com>
> ---
>   drivers/scsi/myrs.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
> index a1eec65a9713..852aceb81f28 100644
> --- a/drivers/scsi/myrs.c
> +++ b/drivers/scsi/myrs.c
> @@ -1408,7 +1408,8 @@ static ssize_t cache_size_show(struct device *dev,
>       struct Scsi_Host *shost = class_to_shost(dev);
>       struct myrs_hba *cs = shost_priv(shost);
> 
> -    return snprintf(buf, 8, "%d MB\n", cs->ctlr_info->cache_size_mb);
> +    return scnprintf(buf, 8, "%d MB\n", cs->ctlr_info->cache_size_mb);
> +

You've added a 'random' blank line.

The '8' ought to be ringing alarm bells as well.

If you actually look at the code I think this should be sysfs_emit().
In any case it looks like someone changed a load of sprintf()
to snprintf() with the expected max size of the output
rather than using a bound for the output buffer.

IIRC at least one of the length is just plain wrong and
leads to valid output being truncated.

Using (say) 256 for all the snprint() would have been
more sensible.

	David

>   }
>   static DEVICE_ATTR_RO(cache_size);

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index a1eec65a9713..852aceb81f28 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -1408,7 +1408,8 @@  static ssize_t cache_size_show(struct device *dev,
      struct Scsi_Host *shost = class_to_shost(dev);
      struct myrs_hba *cs = shost_priv(shost);

-    return snprintf(buf, 8, "%d MB\n", cs->ctlr_info->cache_size_mb);
+    return scnprintf(buf, 8, "%d MB\n", cs->ctlr_info->cache_size_mb);
+
  }
  static DEVICE_ATTR_RO(cache_size);