Re: [PATCH] can: bcm: Replace strcpy() with strscpy(), simplify call

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hello Ethan,

On 19.04.26 04:00, Ethan Carter Edwards wrote:
> strcpy() is deprecated; use the safer strscpy() instead.
> 
> While the src is char array at bcm_proc_getifname's call sites, the src
> is passed to the function as a char*. Including the IFNAMSIZ length
> makes the call safer.

I'm not sure what makes the call "safer" here.

The use of IFNAMSIZ in dev->name and in ifname[IFNAMSIZ] in 
bcm_proc_show() makes it 100% clear that we only can copy data between 
identical sized buffers. Adding strscpy() with IFNAMSIZ is a nop.

The only issue could be that someone reduces IFNAMSIZ to a value smaller 
than sizeof("???") - which is not addressed here.

I see no real benefit changing this code in the proposed way.

Especially as we are not talking about some dynamic content here.

Best regards,
Oliver

> 
> Use turnary over if/else to simplify code.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>
> ---
>   net/can/bcm.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index a4bef2c48a55..c133dab1202e 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -196,10 +196,7 @@ static char *bcm_proc_getifname(struct net *net, char *result, int ifindex)
>   
>   	rcu_read_lock();
>   	dev = dev_get_by_index_rcu(net, ifindex);
> -	if (dev)
> -		strcpy(result, dev->name);
> -	else
> -		strcpy(result, "???");
> +	strscpy(result, dev ? dev->name : "???", IFNAMSIZ);
>   	rcu_read_unlock();
>   
>   	return result;
> 
> ---
> base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
> change-id: 20260418-can-bcm-strscpy-56a0e402a660
> 
> Best regards,