Messages in this thread Patch in this message | | | Subject | Re: [Intel-wired-lan] [net-next] net: iavf: Use the ARRAY_SIZE macro for aq_to_posix | From | Joe Perches <> | Date | Thu, 14 Jan 2021 02:55:13 -0800 |
| |
On Thu, 2021-01-14 at 09:57 +0000, Jankowski, Konrad0 wrote: > > -----Original Message----- > > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Wei Xu [] > > Use the ARRAY_SIZE macro to calculate the size of an array. > > This code was detected with the help of Coccinelle. [] > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h [] > > @@ -120,7 +120,7 @@ static inline int iavf_aq_rc_to_posix(int aq_ret, int aq_rc) > > if (aq_ret == IAVF_ERR_ADMIN_QUEUE_TIMEOUT) > > return -EAGAIN; > > > > - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) > > + if (!((u32)aq_rc < ARRAY_SIZE(aq_to_posix))) > > return -ERANGE; > > > > return aq_to_posix[aq_rc]; > > Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
I think several things are poor here.
This function shouldn't really be a static inline as it would just bloat whatever uses it and should just be a typical function in a utility .c file.
And it doesn't seem this function is used at all so it should be deleted.
aq_to_posix should be static const.
And if it's really necessary, I think it would be simpler to read using code without the cast and negation.
if (aq_rc < 0 || aq_rc >= ARRAY_SIZE(aq_to_posix)) return -ERANGE;
| |