Re: [Intel-wired-lan] [net-next] net: iavf: Use the ARRAY_SIZE macro for aq_to_posix

[Joe Perches]

On Thu, 2021-01-14 at 09:57 +0000, Jankowski, Konrad0 wrote:
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> 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@xxxxxxxxx>

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;