ext: hal: nxp: Use ARRAY_SIZE helper macro

[himanshujha199640]

CC @MaureenHelm

himanshu@himanshu-Vostro-3559:~/zephyr$ ./scripts/coccicheck --cocci=scripts/coccinelle/array_size.cocci --mode=report --verbose=1 ext/hal/nxp/

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting it.

Processing array_size.cocci
with option(s) " --no-includes --include-headers"

Message example to submit a patch:
 Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element

 The semantic patch that makes this report is available
 in scripts/coccinelle/array_size.cocci.

 More information about semantic patching is available at
 http://coccinelle.lip6.fr/

Semantic patch information:
 This makes an effort to find cases where ARRAY_SIZE can be used such as
 where there is a division of sizeof the array by the sizeof its first
 element or by any indexed element or the element type. It replaces the
 division of the two sizeofs by ARRAY_SIZE.

Running (4 in parallel): /usr/local/bin/spatch -D report --no-show-diff --very-quiet --cocci-file scripts/coccinelle/array_size.cocci --no-includes --include-headers --patch /home/himanshu/zephyr --dir ext/hal/nxp/ --jobs 4 --chunksize 1
ext/hal/nxp/mcux/drivers/kinetis/fsl_uart.c:158:49-50: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/kinetis/fsl_ftm.c:86:47-48: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/lpc/fsl_wwdt.c:66:49-50: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/imx/fsl_adc_etc.c:43:53-54: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/kinetis/fsl_ltc.c:3004:45-46: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/kinetis/fsl_i2c.c:208:49-50: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/lpc/fsl_sctimer.c:86:47-48: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/lpc/fsl_mrt.c:65:47-48: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/imx/fsl_dcp.c:928:45-46: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/kinetis/fsl_tpm.c:67:47-48: WARNING: Use ARRAY_SIZE
ext/hal/nxp/imx/drivers/i2c_imx.c:116:56-57: WARNING: Use ARRAY_SIZE
ext/hal/nxp/imx/drivers/i2c_imx.c:119:41-42: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/middleware/wireless/framework_5.3.3/XCVR/MKW41Z4/fsl_xcvr.c:342:59-60: WARNING: Use ARRAY_SIZE
ext/hal/nxp/mcux/drivers/lpc/fsl_ctimer.c:74:53-54: WARNING: Use ARRAY_SIZE

[pfalcon]

@himanshujha199640: You understand that everything in ext/ comes from 3rd-party sources, right? So, it's outside of our control and these reports are effectively "false positives", as the output above says.

[galak]

@himanshujha199640: You understand that everything in ext/ comes from 3rd-party sources, right? So, it's outside of our control and these reports are effectively "false positives", as the output above says.

I suggested that @himanshujha199640 open issues to report what he was seeing so at least there was visibility to those that might care.

[himanshujha199640]

@himanshujha199640: You understand that everything in ext/ comes from 3rd-party sources, right?
So, it's outside of our control and these reports are effectively "false positives", as the output above says.

It doesn't say that the output are false positives but to check before submitting. I checked few
and ARRAY_SIZE seemed like a worthwhile change.

himanshu@himanshu-Vostro-3559:~/zephyr$ ./scripts/coccicheck --cocci=scripts/coccinelle/array_size.cocci --mode=patch --verbose=1 ext/hal/nxp/

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting it.

Processing array_size.cocci
with option(s) " --no-includes --include-headers"

Message example to submit a patch:
 Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element

 The semantic patch that makes this change is available
 in scripts/coccinelle/array_size.cocci.

 More information about semantic patching is available at
 http://coccinelle.lip6.fr/

Semantic patch information:
 This makes an effort to find cases where ARRAY_SIZE can be used such as
 where there is a division of sizeof the array by the sizeof its first
 element or by any indexed element or the element type. It replaces the
 division of the two sizeofs by ARRAY_SIZE.

Running (4 in parallel): /usr/local/bin/spatch -D patch --very-quiet --cocci-file scripts/coccinelle/array_size.cocci --no-includes --include-headers --patch /home/himanshu/zephyr --dir ext/hal/nxp/ --jobs 4 --chunksize 1
diff -u -p a/ext/hal/nxp/mcux/middleware/wireless/framework_5.3.3/XCVR/MKW41Z4/fsl_xcvr.c b/ext/hal/nxp/mcux/middleware/wireless/framework_5.3.3/XCVR/MKW41Z4/fsl_xcvr.c
--- a/ext/hal/nxp/mcux/middleware/wireless/framework_5.3.3/XCVR/MKW41Z4/fsl_xcvr.c
+++ b/ext/hal/nxp/mcux/middleware/wireless/framework_5.3.3/XCVR/MKW41Z4/fsl_xcvr.c
@@ -339,7 +339,7 @@ xcvrStatus_t XCVR_Init(radio_mode_t radi
         {TRIM_STATUS, 0, FALSE}, /*< Fetch the trim status word if available.*/
         {TRIM_VERSION, 0, FALSE} /*< Fetch the trim version number if available.*/
     };
-    const uint8_t NUM_TRIM_TBL_ENTRIES = sizeof(sw_trim_tbl)/sizeof(IFR_SW_TRIM_TBL_ENTRY_T);
+    const uint8_t NUM_TRIM_TBL_ENTRIES = ARRAY_SIZE(sw_trim_tbl);
 
 #ifndef SIMULATION
 
diff -u -p a/ext/hal/nxp/mcux/drivers/kinetis/fsl_tpm.c b/ext/hal/nxp/mcux/drivers/kinetis/fsl_tpm.c
--- a/ext/hal/nxp/mcux/drivers/kinetis/fsl_tpm.c
+++ b/ext/hal/nxp/mcux/drivers/kinetis/fsl_tpm.c
@@ -64,7 +64,7 @@ static const clock_ip_name_t s_tpmClocks
 static uint32_t TPM_GetInstance(TPM_Type *base)
 {
     uint32_t instance;
-    uint32_t tpmArrayCount = (sizeof(s_tpmBases) / sizeof(s_tpmBases[0]));
+    uint32_t tpmArrayCount = ARRAY_SIZE(s_tpmBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < tpmArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/lpc/fsl_sctimer.c b/ext/hal/nxp/mcux/drivers/lpc/fsl_sctimer.c
--- a/ext/hal/nxp/mcux/drivers/lpc/fsl_sctimer.c
+++ b/ext/hal/nxp/mcux/drivers/lpc/fsl_sctimer.c
@@ -83,7 +83,7 @@ static sctimer_isr_t s_sctimerIsr;
 static uint32_t SCTIMER_GetInstance(SCT_Type *base)
 {
     uint32_t instance;
-    uint32_t sctArrayCount = (sizeof(s_sctBases) / sizeof(s_sctBases[0]));
+    uint32_t sctArrayCount = ARRAY_SIZE(s_sctBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < sctArrayCount; instance++)
diff -u -p a/ext/hal/nxp/imx/drivers/i2c_imx.c b/ext/hal/nxp/imx/drivers/i2c_imx.c
--- a/ext/hal/nxp/imx/drivers/i2c_imx.c
+++ b/ext/hal/nxp/imx/drivers/i2c_imx.c
@@ -113,10 +113,10 @@ void I2C_SetBaudRate(I2C_Type* base, uin
         /* If clock divider is too small, using smallest legal divider */
         clkDivIndex = 0;
     }
-    else if (clockDiv > i2cClkDivTab[sizeof(i2cClkDivTab)/sizeof(i2cClkDivTab[0]) - 1][0])
+    else if (clockDiv > i2cClkDivTab[ARRAY_SIZE(i2cClkDivTab) - 1][0])
     {
         /* If clock divider is too large, using largest legal divider */
-        clkDivIndex = sizeof(i2cClkDivTab)/sizeof(i2cClkDivTab[0]) - 1;
+        clkDivIndex = ARRAY_SIZE(i2cClkDivTab) - 1;
     }
     else
     {
diff -u -p a/ext/hal/nxp/mcux/drivers/kinetis/fsl_ltc.c b/ext/hal/nxp/mcux/drivers/kinetis/fsl_ltc.c
--- a/ext/hal/nxp/mcux/drivers/kinetis/fsl_ltc.c
+++ b/ext/hal/nxp/mcux/drivers/kinetis/fsl_ltc.c
@@ -3001,7 +3001,7 @@ status_t LTC_HASH_Init(LTC_Type *base, l
         ltc_memcpy(&ctxInternal->word[kLTC_HashCtxKeyStartIdx], key, keySize);
     }
     ctxInternal->blksz = 0u;
-    for (i = 0; i < sizeof(ctxInternal->blk.w) / sizeof(ctxInternal->blk.w[0]); i++)
+    for (i = 0; i < ARRAY_SIZE(ctxInternal->blk.w); i++)
     {
         ctxInternal->blk.w[0] = 0u;
     }
diff -u -p a/ext/hal/nxp/mcux/drivers/lpc/fsl_wwdt.c b/ext/hal/nxp/mcux/drivers/lpc/fsl_wwdt.c
--- a/ext/hal/nxp/mcux/drivers/lpc/fsl_wwdt.c
+++ b/ext/hal/nxp/mcux/drivers/lpc/fsl_wwdt.c
@@ -63,7 +63,7 @@ static const reset_ip_name_t s_wwdtReset
 static uint32_t WWDT_GetInstance(WWDT_Type *base)
 {
     uint32_t instance;
-    uint32_t wwdtArrayCount = (sizeof(s_wwdtBases) / sizeof(s_wwdtBases[0]));
+    uint32_t wwdtArrayCount = ARRAY_SIZE(s_wwdtBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < wwdtArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/imx/fsl_adc_etc.c b/ext/hal/nxp/mcux/drivers/imx/fsl_adc_etc.c
--- a/ext/hal/nxp/mcux/drivers/imx/fsl_adc_etc.c
+++ b/ext/hal/nxp/mcux/drivers/imx/fsl_adc_etc.c
@@ -40,7 +40,7 @@ static const clock_ip_name_t s_adcetcClo
 static uint32_t ADC_ETC_GetInstance(ADC_ETC_Type *base)
 {
     uint32_t instance = 0U;
-    uint32_t adcetcArrayCount = (sizeof(s_adcetcBases) / sizeof(s_adcetcBases[0]));
+    uint32_t adcetcArrayCount = ARRAY_SIZE(s_adcetcBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < adcetcArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/kinetis/fsl_uart.c b/ext/hal/nxp/mcux/drivers/kinetis/fsl_uart.c
--- a/ext/hal/nxp/mcux/drivers/kinetis/fsl_uart.c
+++ b/ext/hal/nxp/mcux/drivers/kinetis/fsl_uart.c
@@ -155,7 +155,7 @@ static uart_isr_t s_uartIsr;
 uint32_t UART_GetInstance(UART_Type *base)
 {
     uint32_t instance;
-    uint32_t uartArrayCount = (sizeof(s_uartBases) / sizeof(s_uartBases[0]));
+    uint32_t uartArrayCount = ARRAY_SIZE(s_uartBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < uartArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/kinetis/fsl_i2c.c b/ext/hal/nxp/mcux/drivers/kinetis/fsl_i2c.c
--- a/ext/hal/nxp/mcux/drivers/kinetis/fsl_i2c.c
+++ b/ext/hal/nxp/mcux/drivers/kinetis/fsl_i2c.c
@@ -205,7 +205,7 @@ static void I2C_SetHoldTime(I2C_Type *ba
         multiplier = 1u << mult;
 
         /* Scan table to find best match. */
-        for (i = 0u; i < sizeof(s_i2cDividerTable) / sizeof(s_i2cDividerTable[0]); ++i)
+        for (i = 0u; i < ARRAY_SIZE(s_i2cDividerTable); ++i)
         {
             /* Assume SCL hold(stop) value = s_i2cDividerTable[i]/2. */
             computedSclHoldTime = ((multiplier * s_i2cDividerTable[i]) * 500000000U) / sourceClock_Hz;
diff -u -p a/ext/hal/nxp/mcux/drivers/lpc/fsl_mrt.c b/ext/hal/nxp/mcux/drivers/lpc/fsl_mrt.c
--- a/ext/hal/nxp/mcux/drivers/lpc/fsl_mrt.c
+++ b/ext/hal/nxp/mcux/drivers/lpc/fsl_mrt.c
@@ -62,7 +62,7 @@ static const reset_ip_name_t s_mrtResets
 static uint32_t MRT_GetInstance(MRT_Type *base)
 {
     uint32_t instance;
-    uint32_t mrtArrayCount = (sizeof(s_mrtBases) / sizeof(s_mrtBases[0]));
+    uint32_t mrtArrayCount = ARRAY_SIZE(s_mrtBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < mrtArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/kinetis/fsl_ftm.c b/ext/hal/nxp/mcux/drivers/kinetis/fsl_ftm.c
--- a/ext/hal/nxp/mcux/drivers/kinetis/fsl_ftm.c
+++ b/ext/hal/nxp/mcux/drivers/kinetis/fsl_ftm.c
@@ -83,7 +83,7 @@ static const clock_ip_name_t s_ftmClocks
 static uint32_t FTM_GetInstance(FTM_Type *base)
 {
     uint32_t instance;
-    uint32_t ftmArrayCount = (sizeof(s_ftmBases) / sizeof(s_ftmBases[0]));
+    uint32_t ftmArrayCount = ARRAY_SIZE(s_ftmBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < ftmArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/lpc/fsl_ctimer.c b/ext/hal/nxp/mcux/drivers/lpc/fsl_ctimer.c
--- a/ext/hal/nxp/mcux/drivers/lpc/fsl_ctimer.c
+++ b/ext/hal/nxp/mcux/drivers/lpc/fsl_ctimer.c
@@ -71,7 +71,7 @@ static const IRQn_Type s_ctimerIRQ[] = C
 static uint32_t CTIMER_GetInstance(CTIMER_Type *base)
 {
     uint32_t instance;
-    uint32_t ctimerArrayCount = (sizeof(s_ctimerBases) / sizeof(s_ctimerBases[0]));
+    uint32_t ctimerArrayCount = ARRAY_SIZE(s_ctimerBases);
 
     /* Find the instance index from base address mappings. */
     for (instance = 0; instance < ctimerArrayCount; instance++)
diff -u -p a/ext/hal/nxp/mcux/drivers/imx/fsl_dcp.c b/ext/hal/nxp/mcux/drivers/imx/fsl_dcp.c
--- a/ext/hal/nxp/mcux/drivers/imx/fsl_dcp.c
+++ b/ext/hal/nxp/mcux/drivers/imx/fsl_dcp.c
@@ -925,7 +925,7 @@ status_t DCP_HASH_Init(DCP_Type *base, d
     ctxInternal = (dcp_hash_ctx_internal_t *)ctx;
     ctxInternal->algo = algo;
     ctxInternal->blksz = 0u;
-    for (i = 0; i < sizeof(ctxInternal->blk.w) / sizeof(ctxInternal->blk.w[0]); i++)
+    for (i = 0; i < ARRAY_SIZE(ctxInternal->blk.w); i++)
     {
         ctxInternal->blk.w[0] = 0u;
     }

Meanwhile I pused a fix(OpenAMP/libmetal#67) at the "3rd party"
repo when @galak asked me to.

Also, these codesnippets are hard to read due to heavy usage of typedefs.

Anyway, I try to post full command involved to run coccicheck so that it might somehow
interest developers as usually nobody has time to read coccinelle.rst.

I will now stop sending changes for ext/ as it seems uninteresting to the community.

[pfalcon]

@himanshujha199640

I will now stop sending changes for ext/ as it seems uninteresting to the community.

Well, if you had an arrangement with @galak regarding doing that, which has now been explained, then please continue. My note was not to discourage you, but give a warnings that there may be a long path from finding some possible things to improve to them actually being improved.

As for "community", this specific patchset targets internal HAL maintained by NXP. Only NXP representatives may say whether it's useful to them. I personally agree with your analysis and will be thrilled if @MaureenHelm or other NXPers would confirm that the next version of MCUX will come with the changes suggested.

So again, sorry for the false alert, wasn't intended as such.

[himanshujha199640]

@himanshujha199640

I will now stop sending changes for ext/ as it seems uninteresting to the community.

Well, if you had an arrangement with @galak regarding doing that, which has now been explained, then please continue. My note was not to discourage you, but give a warnings that there may be a long path from finding some possible things to improve to them actually being improved.

I'm not discouraged!
It's just that I don't want to waste either of our time pushing issues
which have a low priority.

As for "community", this specific patchset targets internal HAL maintained by NXP. Only NXP representatives may say whether it's useful to them. I personally agree with your analysis and will be thrilled if @MaureenHelm or other NXPers would confirm that the next version of MCUX will come with the changes suggested.

OK.
As @galak pointed to the 3rd party repo, you can also point me to it, and I will push
at that repo instead.

So again, sorry for the false alert, wasn't intended as such.

I work with the linux kernel community, so used to it ;)
https://lore.kernel.org/lkml/[email protected]/

[nashif]

not a zephyr bug.