Messages in this thread | | | Date | Fri, 11 Jun 2021 12:39:59 +0200 | From | Greg KH <> | Subject | Re: [PATCH v2] usbip: tools: usbipd: use ARRAY_SIZE for sockfdlist |
| |
On Fri, Jun 11, 2021 at 06:15:10PM +0800, Jiapeng Chong wrote: > Use ARRAY_SIZE instead of dividing sizeof array with sizeof an > element. > > Clean up the following coccicheck warning: > > ./tools/usb/usbip/src/usbipd.c:536:19-20: WARNING: Use ARRAY_SIZE.
Why are you assuming that coccicheck should be run on userspace code?
> > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com> > --- > Changes in v2: > -Add ARRAY_SIZE definition to usbip_common.h file. > > tools/usb/usbip/libsrc/usbip_common.h | 2 ++ > tools/usb/usbip/src/usbipd.c | 3 +-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_common.h b/tools/usb/usbip/libsrc/usbip_common.h > index 73a367a..4e12dc4 100644 > --- a/tools/usb/usbip/libsrc/usbip_common.h > +++ b/tools/usb/usbip/libsrc/usbip_common.h > @@ -101,6 +101,8 @@ > abort(); \ > } while (0) > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
Why is this needed?
And if it really really is, why not define it in a way that ALL tools can use it.
And then fix it up be correct for cases when you might call this when it is not an array. This is a very naive implementation.
> + > struct usbip_usb_interface { > uint8_t bInterfaceClass; > uint8_t bInterfaceSubClass; > diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c > index 48398a7..4826d13 100644 > --- a/tools/usb/usbip/src/usbipd.c > +++ b/tools/usb/usbip/src/usbipd.c > @@ -532,8 +532,7 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6) > usbip_driver_close(driver); > return -1; > } > - nsockfd = listen_all_addrinfo(ai_head, sockfdlist, > - sizeof(sockfdlist) / sizeof(*sockfdlist)); > + nsockfd = listen_all_addrinfo(ai_head, sockfdlist, ARRAY_SIZE(sockfdlist));
The code here is correct, right? So this is not necessary unless you do this for all in-tree userspace tools at the same time.
thanks,
greg k-h
| |