[PATCH] scsi: hpsa: Use min() to simplify code

[PATCH] scsi: hpsa: Use min() to simplify code

[Thorsten Blum]

Use min() to simplify the host_store_hp_ssd_smart_path_status() and
host_store_raid_offload_debug() functions.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/scsi/hpsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 84d8de07b7ae..1d19eb2ca1d3 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -460,7 +460,7 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct device *dev,
 
 	if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 		return -EACCES;
-	len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
+	len = min(count, sizeof(tmpbuf) - 1);
 	strncpy(tmpbuf, buf, len);
 	tmpbuf[len] = '\0';
 	if (sscanf(tmpbuf, "%d", &status) != 1)
@@ -484,7 +484,7 @@ static ssize_t host_store_raid_offload_debug(struct device *dev,
 
 	if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 		return -EACCES;
-	len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
+	len = min(count, sizeof(tmpbuf) - 1);
 	strncpy(tmpbuf, buf, len);
 	tmpbuf[len] = '\0';
 	if (sscanf(tmpbuf, "%d", &debug_level) != 1)
-- 
2.48.1

Re: [PATCH] scsi: hpsa: Use min() to simplify code

[Bart Van Assche]

On 2/12/25 3:55 AM, Thorsten Blum wrote:
> Use min() to simplify the host_store_hp_ssd_smart_path_status() and
> host_store_raid_offload_debug() functions.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>   drivers/scsi/hpsa.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 84d8de07b7ae..1d19eb2ca1d3 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -460,7 +460,7 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct device *dev,
>   
>   	if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
>   		return -EACCES;
> -	len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
> +	len = min(count, sizeof(tmpbuf) - 1);
>   	strncpy(tmpbuf, buf, len);
>   	tmpbuf[len] = '\0';
>   	if (sscanf(tmpbuf, "%d", &status) != 1)
> @@ -484,7 +484,7 @@ static ssize_t host_store_raid_offload_debug(struct device *dev,
>   
>   	if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
>   		return -EACCES;
> -	len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count;
> +	len = min(count, sizeof(tmpbuf) - 1);
>   	strncpy(tmpbuf, buf, len);
>   	tmpbuf[len] = '\0';
>   	if (sscanf(tmpbuf, "%d", &debug_level) != 1)

 From Documentation/process/deprecated.rst:

<quote>
strncpy() on NUL-terminated strings
-----------------------------------
Use of strncpy() does not guarantee that the destination buffer will
be NUL terminated. This can lead to various linear read overflows and
other misbehavior due to the missing termination. It also NUL-pads
the destination buffer if the source contents are shorter than the
destination buffer size, which may be a needless performance penalty
for callers using only NUL-terminated strings.

When the destination is required to be NUL-terminated, the replacement is
strscpy(), though care must be given to any cases where the return value
of strncpy() was used, since strscpy() does not return a pointer to the
destination, but rather a count of non-NUL bytes copied (or negative
errno when it truncates). Any cases still needing NUL-padding should
instead use strscpy_pad().

If a caller is using non-NUL-terminated strings, strtomem() should be
used, and the destinations should be marked with the `__nonstring
<https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_
attribute to avoid future compiler warnings. For cases still needing
NUL-padding, strtomem_pad() can be used.
</quote>

Instead of only changing the calculation of 'len', please change the
strncpy() calls into strscpy() calls.

Thanks,

Bart.

Re: [PATCH] scsi: hpsa: Use min() to simplify code

[Thorsten Blum]

On 12. Feb 2025, at 19:48, Bart Van Assche wrote:
> On 2/12/25 3:55 AM, Thorsten Blum wrote:
>> Use min() to simplify the host_store_hp_ssd_smart_path_status() and
>> host_store_raid_offload_debug() functions.
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
> 
> From Documentation/process/deprecated.rst:
> 
> <quote>
> strncpy() on NUL-terminated strings
> -----------------------------------
> Use of strncpy() does not guarantee that the destination buffer will
> be NUL terminated. This can lead to various linear read overflows and
> other misbehavior due to the missing termination. It also NUL-pads
> the destination buffer if the source contents are shorter than the
> destination buffer size, which may be a needless performance penalty
> for callers using only NUL-terminated strings.
> 
> When the destination is required to be NUL-terminated, the replacement is
> strscpy(), though care must be given to any cases where the return value
> of strncpy() was used, since strscpy() does not return a pointer to the
> destination, but rather a count of non-NUL bytes copied (or negative
> errno when it truncates). Any cases still needing NUL-padding should
> instead use strscpy_pad().
> 
> If a caller is using non-NUL-terminated strings, strtomem() should be
> used, and the destinations should be marked with the `__nonstring
> <https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_
> attribute to avoid future compiler warnings. For cases still needing
> NUL-padding, strtomem_pad() can be used.
> </quote>
> 
> Instead of only changing the calculation of 'len', please change the
> strncpy() calls into strscpy() calls.

Thank you for the suggestion.

I just sent a new patch [1] essentially replacing this one.

Best,
Thorsten

[1] https://lore.kernel.org/r/20250212222214.86110-2-thorsten.blum@linux.dev/