On Fri, 2018-01-12 at 15:45 +0200, Andy Shevchenko wrote: > On Fri, Jan 12, 2018 at 1:46 PM, Wang <wangxiongfe...@huawei.com> 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.
