From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 350F02BAF for ; Wed, 30 Aug 2017 22:07:32 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Aug 2017 13:07:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,449,1498546800"; d="scan'208";a="146428545" Received: from bricha3-mobl3.ger.corp.intel.com ([10.252.9.42]) by fmsmga006.fm.intel.com with SMTP; 30 Aug 2017 13:07:29 -0700 Received: by (sSMTP sendmail emulation); Wed, 30 Aug 2017 21:07:28 +0100 Date: Wed, 30 Aug 2017 21:07:26 +0100 From: Bruce Richardson To: Jacek Piasecki Cc: dev@dpdk.org, deepak.k.jain@intel.com, kubax.kozak@intel.com, michalx.k.jastrzebski@intel.com Message-ID: <20170830200726.GB1656@bricha3-MOBL3.ger.corp.intel.com> References: <1498560760-104196-2-git-send-email-jacekx.piasecki@intel.com> <1499690657-81150-1-git-send-email-jacekx.piasecki@intel.com> <1499690657-81150-3-git-send-email-jacekx.piasecki@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1499690657-81150-3-git-send-email-jacekx.piasecki@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH v4 2/5] cfgfile: change existing API functions 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: Wed, 30 Aug 2017 20:07:33 -0000 I think the commit title needs rewording. This changes the internals not the API. On Mon, Jul 10, 2017 at 02:44:14PM +0200, Jacek Piasecki wrote: > Change to flat arrays in cfgfile struct force slightly > diffrent data access for most of cfgfile functions. > This patch provides necessary changes in existing API. > > Signed-off-by: Jacek Piasecki > --- Some comments below. I believe the change in return value from -1 to -EINVAL, though a more correct error, actually counts as an ABI change, so I think that should be removed, i.e. keep errors at -1. Once done: Acked-by: Bruce Richardon > lib/librte_cfgfile/rte_cfgfile.c | 120 +++++++++++++++++++-------------------- > lib/librte_cfgfile/rte_cfgfile.h | 6 +- > 2 files changed, 62 insertions(+), 64 deletions(-) > > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c > index c6ae3e3..50fe37a 100644 > --- a/lib/librte_cfgfile/rte_cfgfile.c > +++ b/lib/librte_cfgfile/rte_cfgfile.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > > #include "rte_cfgfile.h" > @@ -42,13 +43,15 @@ > struct rte_cfgfile_section { > char name[CFG_NAME_LEN]; > int num_entries; > - struct rte_cfgfile_entry *entries[0]; > + int allocated_entries; > + struct rte_cfgfile_entry *entries; > }; > > struct rte_cfgfile { > int flags; > int num_sections; > - struct rte_cfgfile_section *sections[0]; > + int allocated_sections; > + struct rte_cfgfile_section *sections; > }; > These are good changes, allowing us to have the sections array and entries arrays separate from the basic data structures. > @@ -409,7 +407,7 @@ rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg, > { > const struct rte_cfgfile_section *s = _get_section(cfg, sectionname); > if (s == NULL) > - return -1; > + return -EINVAL; > return s->num_entries; > } I think this change should be dropped for backward compatibility, or else put in NEXT_ABI #ifdefs and an ABI notice added to the RN.