[dpdk-dev] igb_uio: use macros for array size calculation

  Minor code cleanup.
Remove array size calculations and remove unnecessary assignment.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, March 03, 2016 5:08 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
> 
> Minor code cleanup.
> Remove array size calculations and remove unnecessary assignment.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 3374e44..563c57b 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -58,7 +58,7 @@ struct rte_uio_pci_dev {
>  	enum rte_intr_mode mode;
>  };
> 
> -static char *intr_mode = NULL;
> +static char *intr_mode;
>  static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
> 
>  /* sriov sysfs */
> @@ -332,7 +332,7 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
>  	unsigned long addr, len;
>  	void *internal_addr;
> 
> -	if (sizeof(info->mem) / sizeof(info->mem[0]) <= n)
> +	if (n >= MAX_UIO_MAPS)

Why using hardcoded value is better than sizeof()?
As I can see below there is a macro ARRAY_SIZE, why not to use it here then?
Konstantin

>  		return -EINVAL;
> 
>  	addr = pci_resource_start(dev, pci_bar);
> @@ -357,7 +357,7 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info,
>  {
>  	unsigned long addr, len;
> 
> -	if (sizeof(info->port) / sizeof(info->port[0]) <= n)
> +	if (n >= MAX_UIO_PORT_REGIONS)
>  		return -EINVAL;
> 
>  	addr = pci_resource_start(dev, pci_bar);
> @@ -402,7 +402,7 @@ igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
>  	iom = 0;
>  	iop = 0;
> 
> -	for (i = 0; i != sizeof(bar_names) / sizeof(bar_names[0]); i++) {
> +	for (i = 0; i < ARRAY_SIZE(bar_names); i++) {
>  		if (pci_resource_len(dev, i) != 0 &&
>  				pci_resource_start(dev, i) != 0) {
>  			flags = pci_resource_flags(dev, i);
> --
> 2.5.0
  

On 3/3/2016 5:25 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Thursday, March 03, 2016 5:08 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
>>
>> Minor code cleanup.
>> Remove array size calculations and remove unnecessary assignment.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> index 3374e44..563c57b 100644
>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> @@ -58,7 +58,7 @@ struct rte_uio_pci_dev {
>>  	enum rte_intr_mode mode;
>>  };
>>
>> -static char *intr_mode = NULL;
>> +static char *intr_mode;
>>  static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
>>
>>  /* sriov sysfs */
>> @@ -332,7 +332,7 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
>>  	unsigned long addr, len;
>>  	void *internal_addr;
>>
>> -	if (sizeof(info->mem) / sizeof(info->mem[0]) <= n)
>> +	if (n >= MAX_UIO_MAPS)
> 
> Why using hardcoded value is better than sizeof()?
> As I can see below there is a macro ARRAY_SIZE, why not to use it here then?

Both are valid, but in uio (uio_driver.h) "mem" array defined as:
 struct uio_mem          mem[MAX_UIO_MAPS];

So we already know the size of the array, and it is exposed to us, why
need to calculate. Is there any benefit of calculating it?

Thanks,
ferruh
  

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, March 03, 2016 5:35 PM
> To: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
> 
> On 3/3/2016 5:25 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> >> Sent: Thursday, March 03, 2016 5:08 PM
> >> To: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
> >>
> >> Minor code cleanup.
> >> Remove array size calculations and remove unnecessary assignment.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> >> index 3374e44..563c57b 100644
> >> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> >> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> >> @@ -58,7 +58,7 @@ struct rte_uio_pci_dev {
> >>  	enum rte_intr_mode mode;
> >>  };
> >>
> >> -static char *intr_mode = NULL;
> >> +static char *intr_mode;
> >>  static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
> >>
> >>  /* sriov sysfs */
> >> @@ -332,7 +332,7 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
> >>  	unsigned long addr, len;
> >>  	void *internal_addr;
> >>
> >> -	if (sizeof(info->mem) / sizeof(info->mem[0]) <= n)
> >> +	if (n >= MAX_UIO_MAPS)
> >
> > Why using hardcoded value is better than sizeof()?
> > As I can see below there is a macro ARRAY_SIZE, why not to use it here then?
> 
> Both are valid, but in uio (uio_driver.h) "mem" array defined as:
>  struct uio_mem          mem[MAX_UIO_MAPS];

Yep, so if both are valid, why to change it a the first place? :)

> 
> So we already know the size of the array, and it is exposed to us, why
> need to calculate. Is there any benefit of calculating it?

if in future definition of the mem[] would change to let say:
struct uio_mem   mem[X]
your code would still be valid, and no need to update it. 
Konstantin

> 
> Thanks,
> ferruh
  

On 3/3/2016 5:45 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Thursday, March 03, 2016 5:35 PM
>> To: Ananyev, Konstantin; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
>>
>> On 3/3/2016 5:25 PM, Ananyev, Konstantin wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>>>> Sent: Thursday, March 03, 2016 5:08 PM
>>>> To: dev@dpdk.org
>>>> Subject: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation
>>>>
>>>> Minor code cleanup.
>>>> Remove array size calculations and remove unnecessary assignment.
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>>> index 3374e44..563c57b 100644
>>>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>>> @@ -58,7 +58,7 @@ struct rte_uio_pci_dev {
>>>>  	enum rte_intr_mode mode;
>>>>  };
>>>>
>>>> -static char *intr_mode = NULL;
>>>> +static char *intr_mode;
>>>>  static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX;
>>>>
>>>>  /* sriov sysfs */
>>>> @@ -332,7 +332,7 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
>>>>  	unsigned long addr, len;
>>>>  	void *internal_addr;
>>>>
>>>> -	if (sizeof(info->mem) / sizeof(info->mem[0]) <= n)
>>>> +	if (n >= MAX_UIO_MAPS)
>>>
>>> Why using hardcoded value is better than sizeof()?
>>> As I can see below there is a macro ARRAY_SIZE, why not to use it here then?
>>
>> Both are valid, but in uio (uio_driver.h) "mem" array defined as:
>>  struct uio_mem          mem[MAX_UIO_MAPS];
> 
> Yep, so if both are valid, why to change it a the first place? :)
> 
>>
>> So we already know the size of the array, and it is exposed to us, why
>> need to calculate. Is there any benefit of calculating it?
> 
> if in future definition of the mem[] would change to let say:
> struct uio_mem   mem[X]
> your code would still be valid, and no need to update it. 

Since it is the part of uio API, I expect this will remain same,
otherwise igb_uio.c will already have problems  because there is other
piece of code that already rely on this information.

But I don't have a strong opinion on one or other, I will update this to
use ARRAY_SIZE()


Thanks,
ferruh