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 <[email protected]>
---
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
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;
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))
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Wei Xu
> Sent: ?roda, 9 wrze?nia 2020 10:51
> To: [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Jakub Kicinski
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> 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 <[email protected]>
> ---
> 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 <[email protected]>
---------------------------------------------------------------------
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.
On Thu, 2021-01-14 at 09:57 +0000, Jankowski, Konrad0 wrote:
> > -----Original Message-----
> > From: Intel-wired-lan <[email protected]> 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 <[email protected]>
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;