Re: [PATCH 03/35] fbdev: sisfb: Use safer strscpy() instead of strcpy()

[Helge Deller <deller@gmx.de>]

Hello Ai,

Thanks that you want to contribute!

But your series isn't beneficial in most areas.
Some examples:

On 4/25/26 08:58, Ai Chao wrote:
> Use a safer function strscpy() instead of strcpy() for copying to arrays.
> 
> Only idiomatic code replacement, and no functional changes.
> 
> Signed-off-by: Ai Chao <aichao@kylinos.cn>
> ---
>   drivers/video/fbdev/sis/sis_main.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
> index 84567d67f71d..e87fa261f76c 100644
> --- a/drivers/video/fbdev/sis/sis_main.c
> +++ b/drivers/video/fbdev/sis/sis_main.c
> @@ -205,7 +205,7 @@ static void sisfb_search_mode(char *name, bool quiet)
>   	}
>   
>   	if(strlen(name) <= 19) {
> -		strcpy(strbuf1, name);
> +		strscpy(strbuf1, name);

We have strbuf1[20] above, and the length is checked.
There is no benefit of using strscpy() here.
(The code could be cleaned up in other ways though).

>   		for(i = 0; i < strlen(strbuf1); i++) {
>   			if(strbuf1[i] < '0' || strbuf1[i] > '9') strbuf1[i] = ' ';
>   		}
> @@ -5947,33 +5947,33 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   #ifdef CONFIG_FB_SIS_300
>   		case PCI_DEVICE_ID_SI_730:
>   			ivideo->chip = SIS_730;
> -			strcpy(ivideo->myid, "SiS 730");
> +			strscpy(ivideo->myid, "SiS 730");

The compiler knows at build time the length of myid, and the "SIS 730" string.
Using strscpy() has no benefit here either. Contrary, the code generated
because of using strscpy() is probably even larger.
Don't replace such code with strscpy().


--- a/drivers/video/fbdev/i810/i810-i2c.c
+++ b/drivers/video/fbdev/i810/i810-i2c.c
@@ -91,7 +91,7 @@ static int i810_setup_i2c_bus(struct i810fb_i2c_chan *chan, const char *name)
  {
          int rc;
  
-        strcpy(chan->adapter.name, name);
+	strscpy(chan->adapter.name, name);

Here it might make sense to use strscpy(), but it should be checked manually
and not using scripts to simply replace code.

That said: Thanks for your patches, but as-is I won't take them.

Helge