* [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE [not found] <CGME20260421114034eucas1p1d746ffe32b49aeb57f10e729d1331124@eucas1p1.samsung.com> @ 2026-04-21 11:40 ` Jakub Raczynski 2026-04-21 14:11 ` Dan Carpenter 0 siblings, 1 reply; 7+ messages in thread From: Jakub Raczynski @ 2026-04-21 11:40 UTC (permalink / raw) To: netdev Cc: kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors, Jakub Raczynski There are still places in the code where manual calculation of array size exist, but it is good to enforce usage of single macro through the whole code as it makes code bit more readable. Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> --- drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +- drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h index 1be97a3a86ce..0e7cecd00169 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h @@ -109,7 +109,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc) -EFBIG, /* I40E_AQ_RC_EFBIG */ }; - 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]; diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h b/drivers/net/ethernet/intel/iavf/iavf_adminq.h index bbf5c4b3a2ae..eb0257c86058 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_adminq.h +++ b/drivers/net/ethernet/intel/iavf/iavf_adminq.h @@ -113,7 +113,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.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-21 11:40 ` [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE Jakub Raczynski @ 2026-04-21 14:11 ` Dan Carpenter 2026-04-22 8:32 ` Jakub Raczynski 2026-04-22 10:57 ` [PATCH v2] " Jakub Raczynski 0 siblings, 2 replies; 7+ messages in thread From: Dan Carpenter @ 2026-04-21 14:11 UTC (permalink / raw) To: Jakub Raczynski Cc: netdev, kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors On Tue, Apr 21, 2026 at 01:40:29PM +0200, Jakub Raczynski wrote: > There are still places in the code where manual calculation of array size > exist, but it is good to enforce usage of single macro through the whole > code as it makes code bit more readable. > > Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> > --- > drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +- > drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h > index 1be97a3a86ce..0e7cecd00169 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h > @@ -109,7 +109,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc) > -EFBIG, /* I40E_AQ_RC_EFBIG */ > }; > > - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) > + if (!((u32)aq_rc < ARRAY_SIZE(aq_to_posix))) This still isn't beautiful. There are so many parens. The !(foo < size) formulation is weird. The cast is unnnecessary. Better to write it as: if (aq_rc >= ARRAY_SIZE(aq_to_posix)) return -ERANGE; > return -ERANGE; > > return aq_to_posix[aq_rc]; regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-21 14:11 ` Dan Carpenter @ 2026-04-22 8:32 ` Jakub Raczynski 2026-04-22 13:22 ` Dan Carpenter 2026-04-22 10:57 ` [PATCH v2] " Jakub Raczynski 1 sibling, 1 reply; 7+ messages in thread From: Jakub Raczynski @ 2026-04-22 8:32 UTC (permalink / raw) To: Dan Carpenter Cc: netdev, kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors [-- Attachment #1: Type: text/plain, Size: 898 bytes --] On Tue, Apr 21, 2026 at 05:11:19PM +0300, Dan Carpenter wrote: > On Tue, Apr 21, 2026 at 01:40:29PM +0200, Jakub Raczynski wrote: > > > > - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) > > + if (!((u32)aq_rc < ARRAY_SIZE(aq_to_posix))) > > This still isn't beautiful. There are so many parens. The !(foo < size) > formulation is weird. The cast is unnnecessary. Better to write it as: > > if (aq_rc >= ARRAY_SIZE(aq_to_posix)) > return -ERANGE; > > > return -ERANGE; > > > > return aq_to_posix[aq_rc]; > > regards, > dan carpenter > Alright, will beautify it and resend soon. I can see potential original intention of not comparing unsigned from sizeof with int, maybe that was original compiler configuration to include that warning. But at this variable range it is irrelevant and it is probably most disabled warning ever. regards Jakub Raczynski [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-22 8:32 ` Jakub Raczynski @ 2026-04-22 13:22 ` Dan Carpenter 0 siblings, 0 replies; 7+ messages in thread From: Dan Carpenter @ 2026-04-22 13:22 UTC (permalink / raw) To: Jakub Raczynski Cc: netdev, kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors On Wed, Apr 22, 2026 at 10:32:17AM +0200, Jakub Raczynski wrote: > On Tue, Apr 21, 2026 at 05:11:19PM +0300, Dan Carpenter wrote: > > On Tue, Apr 21, 2026 at 01:40:29PM +0200, Jakub Raczynski wrote: > > > > > > - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) > > > + if (!((u32)aq_rc < ARRAY_SIZE(aq_to_posix))) > > > > This still isn't beautiful. There are so many parens. The !(foo < size) > > formulation is weird. The cast is unnnecessary. Better to write it as: > > > > if (aq_rc >= ARRAY_SIZE(aq_to_posix)) > > return -ERANGE; > > > > > return -ERANGE; > > > > > > return aq_to_posix[aq_rc]; > > > > regards, > > dan carpenter > > > > Alright, will beautify it and resend soon. > > I can see potential original intention of not comparing unsigned from sizeof > with int, maybe that was original compiler configuration to include that > warning. Yeah. I know. I wrote a blog about this... https://staticthinking.wordpress.com/2023/07/25/wsign-compare-is-garbage/ Check this out: $ git grep '(int)' | grep ARRAY_SIZE | wc -l 53 It's as if we want array underflows. > But at this variable range it is irrelevant and it is probably most disabled > warning ever. With the unnecessary cast I had to review to ensure that aq_rc is not unsigned long type. There is a real downside to this kind of rule. regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-21 14:11 ` Dan Carpenter 2026-04-22 8:32 ` Jakub Raczynski @ 2026-04-22 10:57 ` Jakub Raczynski 2026-04-22 13:24 ` Dan Carpenter 2026-04-23 7:41 ` Przemek Kitszel 1 sibling, 2 replies; 7+ messages in thread From: Jakub Raczynski @ 2026-04-22 10:57 UTC (permalink / raw) To: error27 Cc: netdev, kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors, Jakub Raczynski There are still places in the code where manual calculation of array size exist, but it is good to enforce usage of single macro through the whole code as it makes code bit more readable. While at it, beautify condition surrounding it by reversing check and remove unnecessary casting. Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> --- drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +- drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h index 1be97a3a86ce..dcf3baec7b73 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h @@ -109,7 +109,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc) -EFBIG, /* I40E_AQ_RC_EFBIG */ }; - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) + if (aq_rc >= ARRAY_SIZE(aq_to_posix)) return -ERANGE; return aq_to_posix[aq_rc]; diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h b/drivers/net/ethernet/intel/iavf/iavf_adminq.h index bbf5c4b3a2ae..dd2f61172157 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_adminq.h +++ b/drivers/net/ethernet/intel/iavf/iavf_adminq.h @@ -113,7 +113,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 (aq_rc >= ARRAY_SIZE(aq_to_posix)) return -ERANGE; return aq_to_posix[aq_rc]; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-22 10:57 ` [PATCH v2] " Jakub Raczynski @ 2026-04-22 13:24 ` Dan Carpenter 2026-04-23 7:41 ` Przemek Kitszel 1 sibling, 0 replies; 7+ messages in thread From: Dan Carpenter @ 2026-04-22 13:24 UTC (permalink / raw) To: Jakub Raczynski Cc: netdev, kuba, przemyslaw.kitszel, anthony.l.nguyen, kernel-janitors On Wed, Apr 22, 2026 at 12:57:12PM +0200, Jakub Raczynski wrote: > There are still places in the code where manual calculation of array size > exist, but it is good to enforce usage of single macro through the whole > code as it makes code bit more readable. > While at it, beautify condition surrounding it by reversing check and remove > unnecessary casting. > > Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> > --- Thanks. Reviewed-by: Dan Carpenter <error27@gmail.com> regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/intel: Replace manual array size calculation with ARRAY_SIZE 2026-04-22 10:57 ` [PATCH v2] " Jakub Raczynski 2026-04-22 13:24 ` Dan Carpenter @ 2026-04-23 7:41 ` Przemek Kitszel 1 sibling, 0 replies; 7+ messages in thread From: Przemek Kitszel @ 2026-04-23 7:41 UTC (permalink / raw) To: Jakub Raczynski, error27; +Cc: netdev, kuba, anthony.l.nguyen, kernel-janitors On 4/22/26 12:57, Jakub Raczynski wrote: > There are still places in the code where manual calculation of array size > exist, but it is good to enforce usage of single macro through the whole > code as it makes code bit more readable. > While at it, beautify condition surrounding it by reversing check and remove > unnecessary casting. > thank you for the submission, please find some process-related feedback from me for future submissions for intel networking please target IWL (Intel Wired Lan mailing list) patches should be split into per-driver changes most of the time please don't set "In-reply-to: v1" to v2 - just send as a standalone new series (but link to v1 in changelog) this is also a smallest bit above "too trivial to merge" IMO finally this is not -net material, but -next, and -next is closed now for PRs, and this is the only reason that warrants "v3" from you (to:iwl, cc:netdev, after submission window reopens, ~Apr 27th) (please collect Dan's Reviewed-by tag) > Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> > --- > drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +- > drivers/net/ethernet/intel/iavf/iavf_adminq.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h > index 1be97a3a86ce..dcf3baec7b73 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h > +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h > @@ -109,7 +109,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc) > -EFBIG, /* I40E_AQ_RC_EFBIG */ > }; > > - if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))) > + if (aq_rc >= ARRAY_SIZE(aq_to_posix)) > return -ERANGE; > > return aq_to_posix[aq_rc]; > diff --git a/drivers/net/ethernet/intel/iavf/iavf_adminq.h b/drivers/net/ethernet/intel/iavf/iavf_adminq.h > index bbf5c4b3a2ae..dd2f61172157 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_adminq.h > +++ b/drivers/net/ethernet/intel/iavf/iavf_adminq.h > @@ -113,7 +113,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 (aq_rc >= ARRAY_SIZE(aq_to_posix)) > return -ERANGE; > > return aq_to_posix[aq_rc]; ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-23 7:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20260421114034eucas1p1d746ffe32b49aeb57f10e729d1331124@eucas1p1.samsung.com>
2026-04-21 11:40 ` [PATCH] net/intel: Replace manual array size calculation with ARRAY_SIZE Jakub Raczynski
2026-04-21 14:11 ` Dan Carpenter
2026-04-22 8:32 ` Jakub Raczynski
2026-04-22 13:22 ` Dan Carpenter
2026-04-22 10:57 ` [PATCH v2] " Jakub Raczynski
2026-04-22 13:24 ` Dan Carpenter
2026-04-23 7:41 ` Przemek Kitszel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox