Message ID | 1623406510-50900-1-git-send-email-jiapeng.chong@linux.alibaba.com |
---|---|
State | New |
Headers | show |
Series | [v2] usbip: tools: usbipd: use ARRAY_SIZE for sockfdlist | expand |
On 6/11/21 4:39 AM, Greg KH wrote: > 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. > A quick search shows me 53 defines for this across the kernel. Several tools are defining this in their header scope as well as in .c files. There is one in tools/include/linux/kernel.h - the tools that can be compiled from tools/Makefile are okay. However, we have several tools that aren't hooked into tools/Makefile and usbip is one of them. Some tools maintainers probably don't want to add dependency on in-tree header file so they can build it out of tree as a package. At least that might be motivation behind adding the define under the specific tool scope. Not true for usbip - it is something that was overlooked when it was moved from staging to tools. It is a good call to not add one more under this tool header scope and do the cleanup for this properly which would include getting rid of these duplicate defines that are in the kernel now. thanks, -- Shuah
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])) + 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)); freeaddrinfo(ai_head); if (nsockfd <= 0) { err("failed to open a listening socket");
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. 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(-)