[net-next] net: iavf: Use the ARRAY_SIZE macro for aq_to_posix

[net-next] net: iavf: Use the ARRAY_SIZE macro for aq_to_posix

[Wei Xu]

Use the ARRAY_SIZE macro to calculate the size of an array.
This code was detected with the help of Coccinelle.

Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
---
 drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h b/drivers/net/ethernet/intel/iavf/iavf_adminq.h
index baf2fe2..eead12c 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_adminq.h
+++ b/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];
-- 
2.8.1

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

[Joe Perches]

On Wed, 2020-09-09 at 16:51 +0800, Wei Xu wrote:
> 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 b/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;

If you want to use a cast,

	if ((u32)aq_rc >= ARRAY_SIZE(aq_to_posix))
		return -ERANGE;

would be a more common and simpler style, though
perhaps testing ac_rc < 0 would be more intelligible.

	if (ac_rc < 0 || ac_rq >= ARRAY_SIZE(aq_to_posix))
		return -ERANGE;

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

[Joe Perches]

On Wed, 2020-09-09 at 02:33 -0700, Joe Perches wrote:
> On Wed, 2020-09-09 at 16:51 +0800, Wei Xu wrote:
> > 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 b/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;
> 
> If you want to use a cast,
> 
> 	if ((u32)aq_rc >= ARRAY_SIZE(aq_to_posix))
> 		return -ERANGE;
> 
> would be a more common and simpler style, though
> perhaps testing ac_rc < 0 would be more intelligible.
> 
> 	if (ac_rc < 0 || ac_rq >= ARRAY_SIZE(aq_to_posix))

(hah, I typed aq_rc wrong both times, so maybe it's not _that_
 much better...)

	if (aq_rc < 0 || aq_rc >= ARRAY_SIZE(aq_to_posix))

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

[Jankowski, Konrad0]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Wei Xu
> Sent: środa, 9 września 2020 10:51
> To: netdev@vger.kernel.org
> Cc: salil.mehta@huawei.com; jinying@hisilicon.com;
> tangkunshan@huawei.com; huangdaode@hisilicon.com;
> john.garry@huawei.com; linux-kernel@vger.kernel.org;
> linuxarm@huawei.com; shameerali.kolothum.thodi@huawei.com;
> zhangyi.ac@huawei.com; intel-wired-lan@lists.osuosl.org;
> xuwei5@hisilicon.com; jonathan.cameron@huawei.com; Jakub Kicinski
> <kuba@kernel.org>; liguozhu@hisilicon.com; davem@davemloft.net;
> shiju.jose@huawei.com
> Subject: [Intel-wired-lan] [net-next] net: iavf: Use the ARRAY_SIZE macro for
> aq_to_posix
> 
> Use the ARRAY_SIZE macro to calculate the size of an array.
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h
> b/drivers/net/ethernet/intel/iavf/iavf_adminq.h
> index baf2fe2..eead12c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_adminq.h
> +++ b/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>
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
 

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@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;