Re: [PATCH 05/18] net: use ARRAY_SIZE

[Andy Shevchenko]

On Tue, Oct 3, 2017 at 4:22 AM, Jérémy Lefaure
 wrote:
> On Mon, 2 Oct 2017 16:07:36 +0300
> Andy Shevchenko  wrote:
>
>> > +   {_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 
>> > 26, 192,
>> > +32},
>>
>> For all such cases I would rather put on one line disregard checkpatch
>> warning for better readability.
> I agree that it would be better. I didn't know that it was possible to
> not follow this rule for anything else than a string.

IMO, it increases readability quite enough to overrule checkpatch recomendation.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 05/18] net: use ARRAY_SIZE

[Jérémy Lefaure]

On Mon, 02 Oct 2017 16:46:29 +0300
Kalle Valo  wrote:

> We have a tree for wireless so usually it's better to submit wireless
> changes on their own but here I assume Dave will apply this to his tree.
> If not, please resubmit the wireless part in a separate patch.
Ok, I note that.

I'll wait Dave's answer and I'll split this patch if needed.

Thank you,
Jérémy

Re: [PATCH 05/18] net: use ARRAY_SIZE

[Jérémy Lefaure]

On Mon, 2 Oct 2017 16:07:36 +0300
Andy Shevchenko  wrote:

> > +   {_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 26, 
> > 192,
> > +32},  
> 
> For all such cases I would rather put on one line disregard checkpatch
> warning for better readability.
I agree that it would be better. I didn't know that it was possible to
not follow this rule for anything else than a string.

I am waiting for more comments and I will send a v2.

Thank you,
Jérémy

Re: [PATCH 05/18] net: use ARRAY_SIZE

[Kalle Valo]

Jérémy Lefaure  writes:

> Using the ARRAY_SIZE macro improves the readability of the code. Also,
> it is not always useful to use a variable to store this constant
> calculated at compile time.
>
> Found with Coccinelle with the following semantic patch:
> @r depends on (org || report)@
> type T;
> T[] E;
> position p;
> @@
> (
>  (sizeof(E)@p /sizeof(*E))
> |
>  (sizeof(E)@p /sizeof(E[...]))
> |
>  (sizeof(E)@p /sizeof(T))
> )
>
> Signed-off-by: Jérémy Lefaure 
> ---
>  drivers/net/ethernet/emulex/benet/be_cmds.c|   4 +-
>  drivers/net/ethernet/intel/i40e/i40e_adminq.h  |   3 +-
>  drivers/net/ethernet/intel/i40evf/i40e_adminq.h|   3 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c  |   3 +-
>  drivers/net/ethernet/intel/ixgbevf/vf.c|  17 +-
>  drivers/net/usb/kalmia.c   |   9 +-
>  .../broadcom/brcm80211/brcmsmac/phy/phytbl_n.c | 473 
> ++---
>  .../net/wireless/realtek/rtlwifi/rtl8723be/hw.c|   9 +-
>  .../net/wireless/realtek/rtlwifi/rtl8723be/phy.c   |  12 +-
>  .../net/wireless/realtek/rtlwifi/rtl8723be/table.c |  14 +-
>  .../net/wireless/realtek/rtlwifi/rtl8821ae/table.c |  34 +-
>  include/net/bond_3ad.h |   3 +-
>  net/ipv6/seg6_local.c  |   6 +-
>  13 files changed, 177 insertions(+), 413 deletions(-)

We have a tree for wireless so usually it's better to submit wireless
changes on their own but here I assume Dave will apply this to his tree.
If not, please resubmit the wireless part in a separate patch.

-- 
Kalle Valo

Re: [PATCH 05/18] net: use ARRAY_SIZE

[Andy Shevchenko]

On Sun, Oct 1, 2017 at 10:30 PM, Jérémy Lefaure
 wrote:
> Using the ARRAY_SIZE macro improves the readability of the code. Also,
> it is not always useful to use a variable to store this constant
> calculated at compile time.
>

> +   {_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 26, 
> 192,
> +32},

For all such cases I would rather put on one line disregard checkpatch
warning for better readability.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 05/18] net: use ARRAY_SIZE

[Jérémy Lefaure]

Using the ARRAY_SIZE macro improves the readability of the code. Also,
it is not always useful to use a variable to store this constant
calculated at compile time.

Found with Coccinelle with the following semantic patch:
@r depends on (org || report)@
type T;
T[] E;
position p;
@@
(
 (sizeof(E)@p /sizeof(*E))
|
 (sizeof(E)@p /sizeof(E[...]))
|
 (sizeof(E)@p /sizeof(T))
)

Signed-off-by: Jérémy Lefaure 
---
 drivers/net/ethernet/emulex/benet/be_cmds.c|   4 +-
 drivers/net/ethernet/intel/i40e/i40e_adminq.h  |   3 +-
 drivers/net/ethernet/intel/i40evf/i40e_adminq.h|   3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c  |   3 +-
 drivers/net/ethernet/intel/ixgbevf/vf.c|  17 +-
 drivers/net/usb/kalmia.c   |   9 +-
 .../broadcom/brcm80211/brcmsmac/phy/phytbl_n.c | 473 ++---
 .../net/wireless/realtek/rtlwifi/rtl8723be/hw.c|   9 +-
 .../net/wireless/realtek/rtlwifi/rtl8723be/phy.c   |  12 +-
 .../net/wireless/realtek/rtlwifi/rtl8723be/table.c |  14 +-
 .../net/wireless/realtek/rtlwifi/rtl8821ae/table.c |  34 +-
 include/net/bond_3ad.h |   3 +-
 net/ipv6/seg6_local.c  |   6 +-
 13 files changed, 177 insertions(+), 413 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c 
b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 02dd5246dfae..ec39363afd5f 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -15,6 +15,7 @@
  * Costa Mesa, CA 92626
  */
 
+#include 
 #include 
 #include "be.h"
 #include "be_cmds.h"
@@ -103,10 +104,9 @@ static struct be_cmd_priv_map cmd_priv_map[] = {
 static bool be_cmd_allowed(struct be_adapter *adapter, u8 opcode, u8 subsystem)
 {
int i;
-   int num_entries = sizeof(cmd_priv_map)/sizeof(struct be_cmd_priv_map);
u32 cmd_privileges = adapter->cmd_privileges;
 
-   for (i = 0; i < num_entries; i++)
+   for (i = 0; i < ARRAY_SIZE(cmd_priv_map); i++)
if (opcode == cmd_priv_map[i].opcode &&
subsystem == cmd_priv_map[i].subsystem)
if (!(cmd_privileges & cmd_priv_map[i].priv_mask))
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h 
b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
index 2349fbe04bd2..892083b65b91 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
@@ -27,6 +27,7 @@
 #ifndef _I40E_ADMINQ_H_
 #define _I40E_ADMINQ_H_
 
+#include 
 #include "i40e_osdep.h"
 #include "i40e_status.h"
 #include "i40e_adminq_cmd.h"
@@ -143,7 +144,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc)
if (aq_ret == I40E_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];
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h 
b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
index e0bfaa3d4a21..5622a24cc74d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
@@ -27,6 +27,7 @@
 #ifndef _I40E_ADMINQ_H_
 #define _I40E_ADMINQ_H_
 
+#include 
 #include "i40e_osdep.h"
 #include "i40e_status.h"
 #include "i40e_adminq_cmd.h"
@@ -143,7 +144,7 @@ static inline int i40e_aq_rc_to_posix(int aq_ret, int aq_rc)
if (aq_ret == I40E_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];
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
index 19fbb2f28ea4..9cfc8601fb54 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
@@ -21,6 +21,7 @@
  *  Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
  *
  
**/
+#include 
 #include "ixgbe_x540.h"
 #include "ixgbe_type.h"
 #include "ixgbe_common.h"
@@ -947,7 +948,7 @@ static s32 ixgbe_checksum_ptr_x550(struct ixgbe_hw *hw, u16 
ptr,
u16 length, bufsz, i, start;
u16 *local_buffer;
 
-   bufsz = sizeof(buf) / sizeof(buf[0]);
+   bufsz = ARRAY_SIZE(buf);
 
/* Read a chunk at the pointer location */
if (!buffer) {
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c 
b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 0c25006ce9af..96bfef92fb4c 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -24,6 +24,7 @@
 
 
***/
 
+#include