Message ID | 1457024900-18245-2-git-send-email-ferruh.yigit@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id C81BF2BBB; Thu, 3 Mar 2016 18:08:48 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id D8F2A2BA0 for <dev@dpdk.org>; Thu, 3 Mar 2016 18:08:47 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 03 Mar 2016 09:08:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,532,1449561600"; d="scan'208";a="663285596" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by FMSMGA003.fm.intel.com with ESMTP; 03 Mar 2016 09:08:37 -0800 Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com [10.237.217.46]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id u23H8a9i026331; Thu, 3 Mar 2016 17:08:36 GMT Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id u23H8aeM018291; Thu, 3 Mar 2016 17:08:36 GMT Received: (from fyigit@localhost) by sivswdev02.ir.intel.com with id u23H8aJJ018287; Thu, 3 Mar 2016 17:08:36 GMT From: Ferruh Yigit <ferruh.yigit@intel.com> To: dev@dpdk.org Date: Thu, 3 Mar 2016 17:08:20 +0000 Message-Id: <1457024900-18245-2-git-send-email-ferruh.yigit@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1457024900-18245-1-git-send-email-ferruh.yigit@intel.com> References: <1457024900-18245-1-git-send-email-ferruh.yigit@intel.com> Subject: [dpdk-dev] [PATCH] igb_uio: use macros for array size calculation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Ferruh Yigit
March 3, 2016, 5:08 p.m. UTC
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(-)
Comments
> -----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
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) 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);