From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id C66681DB1 for ; Thu, 2 Jul 2015 12:20:20 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 02 Jul 2015 03:20:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,391,1432623600"; d="scan'208";a="598813576" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.61]) by orsmga003.jf.intel.com with SMTP; 02 Jul 2015 03:20:19 -0700 Received: by (sSMTP sendmail emulation); Thu, 02 Jul 2015 11:20:16 +0025 Date: Thu, 2 Jul 2015 11:20:16 +0100 From: Bruce Richardson To: Tetsuya Mukawa Message-ID: <20150702102016.GB3828@bricha3-MOBL3> References: <1435306705-11645-4-git-send-email-mukawa@igel.co.jp> <1435652668-3380-1-git-send-email-mukawa@igel.co.jp> <1435652668-3380-6-git-send-email-mukawa@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1435652668-3380-6-git-send-email-mukawa@igel.co.jp> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v7 05/12] eal: Fix uio mapping differences between linuxapp and bsdapp X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jul 2015 10:20:21 -0000 On Tue, Jun 30, 2015 at 05:24:21PM +0900, Tetsuya Mukawa wrote: > From: "Tetsuya.Mukawa" > > This patch fixes below. > - bsdapp > - Use map_id in pci_uio_map_resource(). > - Fix interface of pci_map_resource(). > - Move path variable of mapped_pci_resource structure to pci_map. > - linuxapp > - Remove redundant error message of linuxapp. > > 'pci_uio_map_resource()' is implemented in both linuxapp and bsdapp, > but interface is different. The patch fixes the function of bsdapp > to do same as linuxapp. After applying it, file descriptor should be > opened and closed out of pci_map_resource(). > > Signed-off-by: Tetsuya Mukawa > --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 118 ++++++++++++++++++------------ > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 21 +++--- > 2 files changed, 80 insertions(+), 59 deletions(-) > > free_uio_res: > + for (i = 0; i < map_idx; i++) > + rte_free(maps[i].path); > rte_free(uio_res); > close_fd: > close(dev->intr_handle.fd); Comments on previous patch about merging error labels would also apply here. > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > index c3b259b..19620fe 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > @@ -116,17 +116,11 @@ pci_uio_map_secondary(struct rte_pci_device *dev) > fd, (off_t)uio_res->maps[i].offset, > (size_t)uio_res->maps[i].size, 0); > if (mapaddr != uio_res->maps[i].addr) { > - if (mapaddr == MAP_FAILED) > - RTE_LOG(ERR, EAL, > - "Cannot mmap device resource file %s: %s\n", > - uio_res->maps[i].path, > - strerror(errno)); > - else > - RTE_LOG(ERR, EAL, > - "Cannot mmap device resource file %s to address: %p\n", > - uio_res->maps[i].path, > - uio_res->maps[i].addr); > - > + RTE_LOG(ERR, EAL, > + "Cannot mmap device resource " > + "file %s to address: %p\n", > + uio_res->maps[i].path, > + uio_res->maps[i].addr); > close(fd); > return -1; > } > @@ -353,8 +347,11 @@ pci_uio_map_resource(struct rte_pci_device *dev) > > /* allocate memory to keep path */ > maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0); > - if (maps[map_idx].path == NULL) > + if (maps[map_idx].path == NULL) { > + RTE_LOG(ERR, EAL, "Cannot allocate memory for " > + "path: %s\n", strerror(errno)); It's recommended not to split literal strings across multiple lines. This is so that it's easy to find error messages in the source code. In this case, because of the split, someone using git grep to search for the error message "Cannot allocate memory for path" would not be able to find it in the code. Longer lines are allowed in code for literal strings. /Bruce