[U-Boot] [PATCH v2] net: Use ARRAY_SIZE at appropriate places

Axel Lin axel.lin at ingics.com
Tue Jul 2 05:45:39 CEST 2013


>>> diff --git a/drivers/net/npe/IxEthDBFeatures.c b/drivers/net/npe/IxEthDBFeatures.c
>>> index c5b680a..d43efaa 100644
>>> --- a/drivers/net/npe/IxEthDBFeatures.c
>>> +++ b/drivers/net/npe/IxEthDBFeatures.c
>>> @@ -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;
>>> +                       }
>>> +               }
>>>
>> Above for loop along with if should be a block separable, means
>>
>> 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.
>

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


More information about the U-Boot mailing list