Re: [PATCH] mptfusion: use strlcpy() instead of strncpy()

[Bart Van Assche <Bart.VanAssche@xxxxxxx>]

On Fri, 2018-01-12 at 15:45 +0200, Andy Shevchenko wrote:
> On Fri, Jan 12, 2018 at 1:46 PM,  Wang <wangxiongfeng2@xxxxxxxxxx> wrote:
> > 
> > -       strncpy (karg->driverVersion, MPT_LINUX_PACKAGE_NAME, MPT_IOCTL_VERSION_LENGTH);
> > -       karg->driverVersion[MPT_IOCTL_VERSION_LENGTH-1]='\0';
> > +       strlcpy (karg->driverVersion, MPT_LINUX_PACKAGE_NAME, MPT_IOCTL_VERSION_LENGTH);
> 
> This one is false positive.
> 
> > -       strncpy (karg.name, ioc->name, MPT_MAX_NAME);
> > -       karg.name[MPT_MAX_NAME-1]='\0';
> > -       strncpy (karg.product, ioc->prod_name, MPT_PRODUCT_LENGTH);
> > -       karg.product[MPT_PRODUCT_LENGTH-1]='\0';
> > +       strlcpy (karg.name, ioc->name, MPT_MAX_NAME);
> > +       strlcpy (karg.product, ioc->prod_name, MPT_PRODUCT_LENGTH);
> 
> These two as well.

But using strlcpy() makes the code easier to read.

Xiongfeng, if you want to continue with this patch, please change the third
argument of all strlcpy() calls into a sizeof() expression. That make the
code easier to verify.

> > -       strncpy(karg.serial_number, " ", 24);
> > +       strlcpy(karg.serial_number, " ", 24);
> 
> This one is interesting indeed.
> Though the fix would be rather something like
> 
> memset(&karg.serial_number, " ", 24); // leave 24 for best performance of memset
> karg.serial_number[24-1] = '\0';

Does performance really matter in this context? How about the following instead:

snprintf(karg.serial_number, sizeof(karg.serial_number), "%*s", (int)(karg.serial_number) - 1, "");

Bart.