From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 64AD47D31 for ; Mon, 4 Sep 2017 18:24:27 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2017 09:24:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,475,1498546800"; d="scan'208";a="125449910" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by orsmga004.jf.intel.com with ESMTP; 04 Sep 2017 09:24:25 -0700 To: Jan Blunck , dev@dpdk.org References: <20170711232512.54641-1-jblunck@infradead.org> <20170714211213.34436-1-jblunck@infradead.org> <20170714211213.34436-9-jblunck@infradead.org> From: Ferruh Yigit Message-ID: <0e436d4d-d9d7-cc53-12e3-c78e56626db9@intel.com> Date: Mon, 4 Sep 2017 17:24:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170714211213.34436-9-jblunck@infradead.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 08/15] devargs: use existing functions in rte_eal_devargs_parse() X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Sep 2017 16:24:27 -0000 On 7/14/2017 10:12 PM, Jan Blunck wrote: > This fixes the newly introduces rte_eal_devargs_parse() to make use of: > - snprintf() instead of open coding a while() loop > - rte_eal_parse_devargs_str() instead of duplicating parsing code > - RTE_LOG() instead of direct output to stderr > > Signed-off-by: Jan Blunck > --- > lib/librte_eal/common/eal_common_devargs.c | 57 +++++++++++++++--------------- > 1 file changed, 28 insertions(+), 29 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c > index 205fabb95..b5273287e 100644 > --- a/lib/librte_eal/common/eal_common_devargs.c > +++ b/lib/librte_eal/common/eal_common_devargs.c > @@ -87,54 +87,53 @@ int > rte_eal_devargs_parse(const char *dev, struct rte_devargs *da) here "const char *dev" is devarg_str, it can be something like: "00:01.0,somekey=somevalue,keymore=valuemore" Calling this "dev" is confusing I think, since you are touching to the function does it make sense to rename this? > { > struct rte_bus *bus = NULL; > - const char *devname; > - const size_t maxlen = sizeof(da->name); > - size_t i; > + char *devname = NULL, *drvargs = NULL; > + int ret; > > if (dev == NULL || da == NULL) > return -EINVAL; > /* Retrieve eventual bus info */ > do { > - devname = dev; > bus = rte_bus_find(bus, bus_name_cmp, dev); > if (bus == NULL) > break; > - devname = dev + strlen(bus->name) + 1; > - if (rte_bus_find_by_device_name(devname) == bus) > + dev += strlen(bus->name) + 1; This deserves a comment I believe, what is done here? > + if (rte_bus_find_by_device_name(dev) == bus) > break; > } while (1); <...>