On 2018/1/16 3:34, Andy Shevchenko wrote: > On Fri, Jan 12, 2018 at 5:09 PM, Bart Van Assche <Bart.VanAssche@xxxxxxx> wrote: >> 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: > >>> This one is false positive. > > >>>> + 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. > > This is another story, not mentioned in the original commit message. > >> 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? > > Nope, but still good to understand this aspect. On some architectures > unaligned access is expensive. > >> How about the following instead: >> >> snprintf(karg.serial_number, sizeof(karg.serial_number), "%*s", (int)(karg.serial_number) - 1, ""); > > ...and you is talking about "easy to read"?! > > So, my view on the patch is: > 1) fix a real issue w/o touching everything around; > 2) (optionally) move to strlcpy() with a correct selling point. > The code is right. There are no issues. It's just that the new compiler gcc-8 prints the warnings. I use strlcpy() instead of strncpy() just to suppress the warnings. Thanks, Xiongfeng
