Skip to contentSkip to conversation

net: Use ARRAY_SIZE at appropriate places

PATCH Superseded Axel Lin 2 participants 4 messages Jun 30, 2013 – Jul 2, 2013 Network drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH v2 1/1 net: Use ARRAY_SIZE at appropriate places

Use ARRAY_SIZE instead of having similar implementation in each drivers.

Signed-off-by Axel Lin <[email protected]>
Cc Albert Aribaud <[email protected]>
Cc Ben Warren <[email protected]>
Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Joe Hershberger <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Nobuhiro Iwamatsu <[email protected]>
Cc TsiChungLiew <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc York Sun <[email protected]>
Signed-off-by Axel Lin <[email protected]>

---
Hi Jagan Teki,
This is v2 per your request.

v2:
Fix checkpatch issues, now checkpatch only shows below warnings:
a few WARNING: Avoid CamelCase
and a WARNING: line over 80 characters
#134: FILE: drivers/net/npe/IxEthDBFeatures.c:150:
+ if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {

Obviously, I prefer v1 for below reasons:
1. The diff in v2 becomes bigger and makes it harder for review.
2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, only the lines touched by this patch uses tab as indent (to make checkpatch happy). This makes the code looks odd after apply v2, I don't think this is an improvement.

Well, it's up to maintainer to pick up the version he prefer.

Regards,
Axel

 drivers/net/ax88180.c                     |  2 +-
 drivers/net/fsl_mcdmafec.c                |  2 +-
 drivers/net/lan91c96.c                    |  2 +-
 drivers/net/mcffec.c                      |  2 +-
 drivers/net/mcfmii.c                      |  2 +-
 drivers/net/ne2000.c                      |  2 +-
 drivers/net/npe/IxEthDBFeatures.c         | 28 ++++++++++++++--------------
 drivers/net/npe/IxOsalIoMem.c             |  3 +--
 drivers/net/npe/include/IxEthDBPortDefs.h |  2 +-
 drivers/net/npe/include/IxOsalTypes.h     |  2 +-
 10 files changed, 23 insertions(+), 24 deletions(-)
drivers/net/ax88180.c +1 -1
index f501768..7f0cfe5 100644
@@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
 	OUTW (dev, MISC_RESET_MAC, MISC);
 	tmpval = INW (dev, MISC);
 
-	for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
+	for (i = 0; i < ARRAY_SIZE(program_seq); i++)
 		OUTW (dev, program_seq[i].value, program_seq[i].offset);
 }
drivers/net/fsl_mcdmafec.c +1 -1
index 63842cd..0e18764 100644
@@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
 	u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
 #endif
 
-	for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+	for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
 		dev =
 		    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/lan91c96.c +1 -1
index 11d350e..47c15c4 100644
@@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
 	SMC_SELECT_BANK(dev, 3);
 	chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
 	SMC_SELECT_BANK(dev, 0);
-	for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
+	for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
 		if (chip_id == supported_chips[r].id)
 			return r;
 	return 0;
drivers/net/mcffec.c +1 -1
index ed7459c..7ae6320 100644
@@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
 	u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
 #endif
 
-	for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+	for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
 		dev =
 		    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/mcfmii.c +1 -1
index 5e64dbd..63bc8f8 100644
@@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
 			printf("PHY @ 0x%x pass %d\n", phyno, pass);
 #endif
 
-			for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t)))
+			for (i = 0; (i < ARRAY_SIZE(phyinfo))
 				&& (phyinfo[i].phyid != 0); i++) {
 				if (phyinfo[i].phyid == phytype) {
 #ifdef ET_DEBUG
drivers/net/ne2000.c +1 -1
index 3939158..e6cd3e9 100644
@@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
 
 	mdelay (10);
 
-	for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
+	for (i = 0; i < ARRAY_SIZE(program_seq); i++)
 		n2k_outb (program_seq[i].value, program_seq[i].offset);
 
 	PRINTK ("PROM:");
drivers/net/npe/IxEthDBFeatures.c +14 -14
index c5b680a..d43efaa 100644
@@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void)
                 IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h");
 
                 /* find the traffic class definition index compatible with the current NPE A functionality ID */
-                for (trafficClassDefinitionIndex = 0 ;
-                    trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]);
-                    trafficClassDefinitionIndex++)
-                {
-                    if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId)
-                    {
-                        /* found it */
-                        break;
-                    }
-                }
+		for (trafficClassDefinitionIndex = 0;
+			trafficClassDefinitionIndex <
+			ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
+			trafficClassDefinitionIndex++) {
+			if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {
+				/* found it */
+				break;
+			}
+		}
 
                 /* select the default case if we went over the array boundary */
-                if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]))
-                {
-                    trafficClassDefinitionIndex = 0; /* the first record is the default case */
-                }
+		if (trafficClassDefinitionIndex ==
+			ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) {
+			/* the first record is the default case */
+			trafficClassDefinitionIndex = 0;
+		}
 
                 /* select queue assignment structure based on the traffic class configuration index */
                 queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX];
drivers/net/npe/IxOsalIoMem.c +1 -2
index 34df92b..15a16b8 100644
@@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress,
 {
     UINT32 mapIndex;
 
-    UINT32 numMapElements =
-        sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap);
+	UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap);
 
     for (mapIndex = 0; mapIndex < numMapElements; mapIndex++)
     {
drivers/net/npe/include/IxEthDBPortDefs.h +1 -1
index c3acbdd..eac48cb 100644
@@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] =
  * @def IX_ETH_DB_NUMBER_OF_PORTS
  * @brief number of supported ports 
  */
-#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0]))
+#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions)
 
 /**
  * @def IX_ETH_DB_UNKNOWN_PORT
drivers/net/npe/include/IxOsalTypes.h +1 -1
index 06e71de..615c655 100644
@@ -93,7 +93,7 @@ typedef volatile INT32 VINT32;
 
 
 #ifndef NUMELEMS
-#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0]))
+#define NUMELEMS(x) ARRAY_SIZE(x)
 #endif
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH v2 1/1 net: Use ARRAY_SIZE at appropriate places

Hi Axel,

Thanks for your v2.

Quoted 19 lines · Axel Lin

Use ARRAY_SIZE instead of having similar implementation in each drivers.

Signed-off-by Axel Lin <[email protected]>
Cc Albert Aribaud <[email protected]>
Cc Ben Warren <[email protected]>
Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Joe Hershberger <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Nobuhiro Iwamatsu <[email protected]>
Cc TsiChungLiew <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc York Sun <[email protected]>
Signed-off-by Axel Lin <[email protected]>

---
Hi Jagan Teki,
This is v2 per your request.

Quoted 6 lines

v2:
Fix checkpatch issues, now checkpatch only shows below warnings:
a few WARNING: Avoid CamelCase
and a WARNING: line over 80 characters
#134: FILE: drivers/net/npe/IxEthDBFeatures.c:150:
+ if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {

I think you can fix this warning and may be you can send a separate patch for "CamelCase:"
if you want.

And also..

Quoted 120 lines

Obviously, I prefer v1 for below reasons:
1. The diff in v2 becomes bigger and makes it harder for review.
2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, only the lines touched by this patch uses tab as indent (to make checkpatch happy). This makes the code looks odd after apply v2, I don't think this is an improvement.

Well, it's up to maintainer to pick up the version he prefer.

Regards,
Axel

 drivers/net/ax88180.c                     |  2 +-
 drivers/net/fsl_mcdmafec.c                |  2 +-
 drivers/net/lan91c96.c                    |  2 +-
 drivers/net/mcffec.c                      |  2 +-
 drivers/net/mcfmii.c                      |  2 +-
 drivers/net/ne2000.c                      |  2 +-
 drivers/net/npe/IxEthDBFeatures.c         | 28 ++++++++++++++--------------
 drivers/net/npe/IxOsalIoMem.c             |  3 +--
 drivers/net/npe/include/IxEthDBPortDefs.h |  2 +-
 drivers/net/npe/include/IxOsalTypes.h     |  2 +-
 10 files changed, 23 insertions(+), 24 deletions(-)
drivers/net/ax88180.c +1 -1
index f501768..7f0cfe5 100644
@@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
        OUTW (dev, MISC_RESET_MAC, MISC);
        tmpval = INW (dev, MISC);
 
-       for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
+       for (i = 0; i < ARRAY_SIZE(program_seq); i++)
                OUTW (dev, program_seq[i].value, program_seq[i].offset);
 }
drivers/net/fsl_mcdmafec.c +1 -1
index 63842cd..0e18764 100644
@@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
        u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
 #endif
 
-       for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+       for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
                dev =
                    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/lan91c96.c +1 -1
index 11d350e..47c15c4 100644
@@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
        SMC_SELECT_BANK(dev, 3);
        chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
        SMC_SELECT_BANK(dev, 0);
-       for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
+       for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
                if (chip_id == supported_chips[r].id)
                        return r;
        return 0;
drivers/net/mcffec.c +1 -1
index ed7459c..7ae6320 100644
@@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
        u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
 #endif
 
-       for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+       for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
                dev =
                    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/mcfmii.c +1 -1
index 5e64dbd..63bc8f8 100644
@@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
                        printf("PHY @ 0x%x pass %d\n", phyno, pass);
 #endif
 
-                       for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t)))
+                       for (i = 0; (i < ARRAY_SIZE(phyinfo))
                                && (phyinfo[i].phyid != 0); i++) {
                                if (phyinfo[i].phyid == phytype) {
 #ifdef ET_DEBUG
drivers/net/ne2000.c +1 -1
index 3939158..e6cd3e9 100644
@@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
 
        mdelay (10);
 
-       for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
+       for (i = 0; i < ARRAY_SIZE(program_seq); i++)
                n2k_outb (program_seq[i].value, program_seq[i].offset);
 
        PRINTK ("PROM:");
drivers/net/npe/IxEthDBFeatures.c +0 -10
index c5b680a..d43efaa 100644
@@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void)
                 IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h");
 
                 /* find the traffic class definition index compatible with the current NPE A functionality ID */
-                for (trafficClassDefinitionIndex = 0 ;
-                    trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]);
-                    trafficClassDefinitionIndex++)
-                {
-                    if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId)
-                    {
-                        /* found it */
-                        break;
-                    }
-                }

drivers/net/npe/IxEthDBFeatures.c void ixEthDBFeatureCapabilityScan(void) · line 155

Quoted 10 lines
drivers/net/npe/IxEthDBFeatures.c +9 -0
+               for (trafficClassDefinitionIndex = 0;
+                       trafficClassDefinitionIndex <
+                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
+                       trafficClassDefinitionIndex++) {
+                       if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {
+                               /* found it */
+                               break;
+                       }
+               }

Above for loop along with if should be a block separable, means

for (trafficClassDefinitionIndex = 0;
                 trafficClassDefinitionIndex <
                 ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
                 trafficClassDefinitionIndex++) {
                 if (..........) {

--
Thanks,
Jagan.

Quoted 58 lines
drivers/net/npe/IxEthDBFeatures.c +5 -4
                 /* select the default case if we went over the array boundary */
-                if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]))
-                {
-                    trafficClassDefinitionIndex = 0; /* the first record is the default case */
-                }
+               if (trafficClassDefinitionIndex ==
+                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) {
+                       /* the first record is the default case */
+                       trafficClassDefinitionIndex = 0;
+               }
 
                 /* select queue assignment structure based on the traffic class configuration index */
                 queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX];
drivers/net/npe/IxOsalIoMem.c +1 -2
index 34df92b..15a16b8 100644
@@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress,
 {
     UINT32 mapIndex;
 
-    UINT32 numMapElements =
-        sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap);
+       UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap);
 
     for (mapIndex = 0; mapIndex < numMapElements; mapIndex++)
     {
drivers/net/npe/include/IxEthDBPortDefs.h +1 -1
index c3acbdd..eac48cb 100644
@@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] =
  * @def IX_ETH_DB_NUMBER_OF_PORTS
  * @brief number of supported ports
  */
-#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0]))
+#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions)
 
 /**
  * @def IX_ETH_DB_UNKNOWN_PORT
drivers/net/npe/include/IxOsalTypes.h +1 -2
index 06e71de..615c655 100644
@@ -93,7 +93,7 @@ typedef volatile INT32 VINT32;
 
 
 #ifndef NUMELEMS
-#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0]))
+#define NUMELEMS(x) ARRAY_SIZE(x)
 #endif
 
 
--

1.8.1.2

#2 lore raw

PATCH v2 1/1 net: Use ARRAY_SIZE at appropriate places

Quoted 169 lines · Jagan Teki

Hi Axel,

Thanks for your v2.

Quoted 19 lines · Axel Lin

Use ARRAY_SIZE instead of having similar implementation in each drivers.

Signed-off-by Axel Lin <[email protected]>
Cc Albert Aribaud <[email protected]>
Cc Ben Warren <[email protected]>
Cc Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Joe Hershberger <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Mike Frysinger <[email protected]>
Cc Nobuhiro Iwamatsu <[email protected]>
Cc TsiChungLiew <[email protected]>
Cc Wolfgang Denk <[email protected]>
Cc York Sun <[email protected]>
Signed-off-by Axel Lin <[email protected]>

---
Hi Jagan Teki,
This is v2 per your request.

Quoted 6 lines

v2:
Fix checkpatch issues, now checkpatch only shows below warnings:
a few WARNING: Avoid CamelCase
and a WARNING: line over 80 characters
#134: FILE: drivers/net/npe/IxEthDBFeatures.c:150:
+ if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {

I think you can fix this warning and may be you can send a separate patch for "CamelCase:"
if you want.

And also..

Quoted 120 lines

Obviously, I prefer v1 for below reasons:
1. The diff in v2 becomes bigger and makes it harder for review.
2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, only the lines touched by this patch uses tab as indent (to make checkpatch happy). This makes the code looks odd after apply v2, I don't think this is an improvement.

Well, it's up to maintainer to pick up the version he prefer.

Regards,
Axel

 drivers/net/ax88180.c                     |  2 +-
 drivers/net/fsl_mcdmafec.c                |  2 +-
 drivers/net/lan91c96.c                    |  2 +-
 drivers/net/mcffec.c                      |  2 +-
 drivers/net/mcfmii.c                      |  2 +-
 drivers/net/ne2000.c                      |  2 +-
 drivers/net/npe/IxEthDBFeatures.c         | 28 ++++++++++++++--------------
 drivers/net/npe/IxOsalIoMem.c             |  3 +--
 drivers/net/npe/include/IxEthDBPortDefs.h |  2 +-
 drivers/net/npe/include/IxOsalTypes.h     |  2 +-
 10 files changed, 23 insertions(+), 24 deletions(-)
drivers/net/ax88180.c +1 -1
index f501768..7f0cfe5 100644
@@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
        OUTW (dev, MISC_RESET_MAC, MISC);
        tmpval = INW (dev, MISC);
 
-       for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
+       for (i = 0; i < ARRAY_SIZE(program_seq); i++)
                OUTW (dev, program_seq[i].value, program_seq[i].offset);
 }
drivers/net/fsl_mcdmafec.c +1 -1
index 63842cd..0e18764 100644
@@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
        u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
 #endif
 
-       for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+       for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
                dev =
                    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/lan91c96.c +1 -1
index 11d350e..47c15c4 100644
@@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
        SMC_SELECT_BANK(dev, 3);
        chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
        SMC_SELECT_BANK(dev, 0);
-       for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
+       for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
                if (chip_id == supported_chips[r].id)
                        return r;
        return 0;
drivers/net/mcffec.c +1 -1
index ed7459c..7ae6320 100644
@@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
        u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
 #endif
 
-       for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+       for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
                dev =
                    (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
drivers/net/mcfmii.c +1 -1
index 5e64dbd..63bc8f8 100644
@@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
                        printf("PHY @ 0x%x pass %d\n", phyno, pass);
 #endif
 
-                       for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t)))
+                       for (i = 0; (i < ARRAY_SIZE(phyinfo))
                                && (phyinfo[i].phyid != 0); i++) {
                                if (phyinfo[i].phyid == phytype) {
 #ifdef ET_DEBUG
drivers/net/ne2000.c +1 -1
index 3939158..e6cd3e9 100644
@@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
 
        mdelay (10);
 
-       for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
+       for (i = 0; i < ARRAY_SIZE(program_seq); i++)
                n2k_outb (program_seq[i].value, program_seq[i].offset);
 
        PRINTK ("PROM:");
drivers/net/npe/IxEthDBFeatures.c +0 -10
index c5b680a..d43efaa 100644
@@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void)
                 IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h");
 
                 /* find the traffic class definition index compatible with the current NPE A functionality ID */
-                for (trafficClassDefinitionIndex = 0 ;
-                    trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]);
-                    trafficClassDefinitionIndex++)
-                {
-                    if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId)
-                    {
-                        /* found it */
-                        break;
-                    }
-                }

drivers/net/npe/IxEthDBFeatures.c void ixEthDBFeatureCapabilityScan(void) · line 155

Quoted 10 lines
drivers/net/npe/IxEthDBFeatures.c +9 -0
+               for (trafficClassDefinitionIndex = 0;
+                       trafficClassDefinitionIndex <
+                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
+                       trafficClassDefinitionIndex++) {
+                       if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {
+                               /* found it */
+                               break;
+                       }
+               }

Above for loop along with if should be a block separable, means

Quoted 5 lines
for (trafficClassDefinitionIndex = 0;
                 trafficClassDefinitionIndex <
                 ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
                 trafficClassDefinitionIndex++) {
                 if (..........) {

Previous msg sent intermediately,

See my exact comment here.

for (trafficClassDefinitionIndex = 0;
                       trafficClassDefinitionIndex <
                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
                       trafficClassDefinitionIndex++) {
           if (..........) {

The above way is easy to understand where the code block starts, It just my opinion you may find the different way
but it should be understandable where should code block start for a given condition statement.

--
Thanks,
Jagan.

Quoted 64 lines

--
Thanks,
Jagan.

Quoted 58 lines
drivers/net/npe/IxEthDBFeatures.c +5 -4
                 /* select the default case if we went over the array boundary */
-                if (trafficClassDefinitionIndex == sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]))
-                {
-                    trafficClassDefinitionIndex = 0; /* the first record is the default case */
-                }
+               if (trafficClassDefinitionIndex ==
+                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions)) {
+                       /* the first record is the default case */
+                       trafficClassDefinitionIndex = 0;
+               }
 
                 /* select queue assignment structure based on the traffic class configuration index */
                 queueStructureIndex = ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_QUEUE_ASSIGNMENT_INDEX];
drivers/net/npe/IxOsalIoMem.c +1 -2
index 34df92b..15a16b8 100644
@@ -66,8 +66,7 @@ ixOsalMemMapFind (UINT32 requestedAddress,
 {
     UINT32 mapIndex;
 
-    UINT32 numMapElements =
-        sizeof (ixOsalGlobalMemoryMap) / sizeof (IxOsalMemoryMap);
+       UINT32 numMapElements = ARRAY_SIZE(ixOsalGlobalMemoryMap);
 
     for (mapIndex = 0; mapIndex < numMapElements; mapIndex++)
     {
drivers/net/npe/include/IxEthDBPortDefs.h +1 -1
index c3acbdd..eac48cb 100644
@@ -106,7 +106,7 @@ static const IxEthDBPortDefinition ixEthDBPortDefinitions[] =
  * @def IX_ETH_DB_NUMBER_OF_PORTS
  * @brief number of supported ports
  */
-#define IX_ETH_DB_NUMBER_OF_PORTS (sizeof (ixEthDBPortDefinitions) / sizeof (ixEthDBPortDefinitions[0]))
+#define IX_ETH_DB_NUMBER_OF_PORTS ARRAY_SIZE(ixEthDBPortDefinitions)
 
 /**
  * @def IX_ETH_DB_UNKNOWN_PORT
drivers/net/npe/include/IxOsalTypes.h +1 -2
index 06e71de..615c655 100644
@@ -93,7 +93,7 @@ typedef volatile INT32 VINT32;
 
 
 #ifndef NUMELEMS
-#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0]))
+#define NUMELEMS(x) ARRAY_SIZE(x)
 #endif
 
 
--

1.8.1.2

author
#3 lore raw

PATCH v2 1/1 net: Use ARRAY_SIZE at appropriate places

Quoted 46 lines
Quoted 30 lines
Quoted 18 lines
drivers/net/npe/IxEthDBFeatures.c +0 -10
index c5b680a..d43efaa 100644
@@ -143,22 +143,22 @@ void ixEthDBFeatureCapabilityScan(void)
                 IX_ENSURE(sizeof (ixEthDBTrafficClassDefinitions) != 0, "DB: no traffic class definitions found, check IxEthDBQoS.h");
 
                 /* find the traffic class definition index compatible with the current NPE A functionality ID */
-                for (trafficClassDefinitionIndex = 0 ;
-                    trafficClassDefinitionIndex < sizeof (ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]);
-                    trafficClassDefinitionIndex++)
-                {
-                    if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId)
-                    {
-                        /* found it */
-                        break;
-                    }
-                }

drivers/net/npe/IxEthDBFeatures.c void ixEthDBFeatureCapabilityScan(void) · line 155

Quoted 10 lines
drivers/net/npe/IxEthDBFeatures.c +9 -0
+               for (trafficClassDefinitionIndex = 0;
+                       trafficClassDefinitionIndex <
+                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
+                       trafficClassDefinitionIndex++) {
+                       if (ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX] == npeAImageId.functionalityId) {
+                               /* found it */
+                               break;
+                       }
+               }

Above for loop along with if should be a block separable, means

Quoted 5 lines
for (trafficClassDefinitionIndex = 0;
                 trafficClassDefinitionIndex <
                 ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
                 trafficClassDefinitionIndex++) {
                 if (..........) {

Previous msg sent intermediately,

See my exact comment here.

for (trafficClassDefinitionIndex = 0;
                       trafficClassDefinitionIndex <
                       ARRAY_SIZE(ixEthDBTrafficClassDefinitions);
                       trafficClassDefinitionIndex++) {
           if (..........) {
Quoted 5 lines

The above way is easy to understand where the code block starts, It just my opinion you may find the different way
but it should be understandable where should code block start for a given condition statement.

Hi Jagan,

I thogut I just proved v2 is worse than v1, but obviously you don't think so.

In v1:

 drivers/net/npe/IxEthDBFeatures.c         | 4 ++--
In v2:
 drivers/net/npe/IxEthDBFeatures.c         | 28 ++++++++++++++--------------

The intention of this patch is really simple and clear in v1 - to use ARRAY_SIZE.

Note, the checkpatch issue is not introduced by this patch. It is because the original file does not pass checkpath at all.

If you do apply v2 to you should be able to see my comment in v2:
The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent, only the lines touched by this patch uses tab as indent (to make checkpatch happy).
Note: tab takes 8 spaces. The indent looks really odd after apply v2.

I just cannot convince myself v2 is better.
I do think we should use v1 and you can send a separate patch to fix all checkpatch warnings if you want.

Regards,
Axel