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 0D4B82C37 for ; Fri, 30 Jun 2017 13:18:37 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jun 2017 04:18:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,286,1496127600"; d="scan'208";a="119348889" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.28]) by orsmga005.jf.intel.com with SMTP; 30 Jun 2017 04:18:33 -0700 Received: by (sSMTP sendmail emulation); Fri, 30 Jun 2017 12:18:33 +0100 Date: Fri, 30 Jun 2017 12:18:33 +0100 From: Bruce Richardson To: Jacek Piasecki Cc: dev@dpdk.org, deepak.k.jain@intel.com, Kuba Kozak Message-ID: <20170630111832.GE14776@bricha3-MOBL3.ger.corp.intel.com> References: <1498474759-102089-2-git-send-email-jacekx.piasecki@intel.com> <1498559210-104084-1-git-send-email-jacekx.piasecki@intel.com> <1498559210-104084-4-git-send-email-jacekx.piasecki@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1498559210-104084-4-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.1 (2017-04-11) Subject: Re: [dpdk-dev] [PATCH v3 3/4] cfgfile: rework of load function 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: Fri, 30 Jun 2017 11:18:38 -0000 On Tue, Jun 27, 2017 at 12:26:49PM +0200, Jacek Piasecki wrote: > From: Kuba Kozak > > New functions added to cfgfile library make it possible > to significantly simplify the code of rte_cfgfile_load_with_params() > > This patch shows the new body of this function. > > Signed-off-by: Jacek Piasecki > --- > lib/librte_cfgfile/rte_cfgfile.c | 143 +++++---------------------------------- > 1 file changed, 17 insertions(+), 126 deletions(-) > > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c > index 518b6ab..5625c80 100644 > --- a/lib/librte_cfgfile/rte_cfgfile.c > +++ b/lib/librte_cfgfile/rte_cfgfile.c > @@ -207,10 +207,6 @@ struct rte_cfgfile * > rte_cfgfile_load_with_params(const char *filename, int flags, > const struct rte_cfgfile_parameters *params) > { > - int allocated_sections = CFG_ALLOC_SECTION_BATCH; > - int allocated_entries = 0; > - int curr_section = -1; > - int curr_entry = -1; > char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0}; > int lineno = 0; > struct rte_cfgfile *cfg = NULL; > @@ -222,28 +218,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags, > if (f == NULL) > return NULL; > > - cfg = malloc(sizeof(*cfg) + sizeof(cfg->sections[0]) * > - allocated_sections); > - if (cfg == NULL) > - goto error2; > - > - memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections); > - > - if (flags & CFG_FLAG_GLOBAL_SECTION) { > - curr_section = 0; > - allocated_entries = CFG_ALLOC_ENTRY_BATCH; > - cfg->sections[curr_section] = malloc( > - sizeof(*cfg->sections[0]) + > - sizeof(cfg->sections[0]->entries[0]) * > - allocated_entries); > - if (cfg->sections[curr_section] == NULL) { > - printf("Error - no memory for global section\n"); > - goto error1; > - } > - > - snprintf(cfg->sections[curr_section]->name, > - sizeof(cfg->sections[0]->name), "GLOBAL"); > - } > + cfg = rte_cfgfile_create(flags); > > while (fgets(buffer, sizeof(buffer), f) != NULL) { > char *pos = NULL; > @@ -254,6 +229,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags, > "Check if line too long\n", lineno); > goto error1; > } > + /* skip parsing if comment character found */ > pos = memchr(buffer, params->comment_character, len); > if (pos != NULL) { > *pos = '\0'; > @@ -261,6 +237,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags, > } > > len = _strip(buffer, len); > + /* skip lines without useful content */ > if (buffer[0] != '[' && memchr(buffer, '=', len) == NULL) > continue; > > @@ -268,130 +245,44 @@ rte_cfgfile_load_with_params(const char *filename, int flags, > /* section heading line */ > char *end = memchr(buffer, ']', len); > if (end == NULL) { > - printf("Error line %d - no terminating '['" > + printf("Error line %d - no terminating ']'" > "character found\n", lineno); > goto error1; > } > *end = '\0'; > _strip(&buffer[1], end - &buffer[1]); > > - /* close off old section and add start new one */ > - if (curr_section >= 0) > - cfg->sections[curr_section]->num_entries = > - curr_entry + 1; > - curr_section++; > - > - /* resize overall struct if we don't have room for more > - sections */ > - if (curr_section == allocated_sections) { > - allocated_sections += CFG_ALLOC_SECTION_BATCH; > - struct rte_cfgfile *n_cfg = realloc(cfg, > - sizeof(*cfg) + sizeof(cfg->sections[0]) > - * allocated_sections); > - if (n_cfg == NULL) { > - curr_section--; > - printf("Error - no more memory\n"); > - goto error1; > - } > - cfg = n_cfg; > - } > - > - /* allocate space for new section */ > - allocated_entries = CFG_ALLOC_ENTRY_BATCH; > - curr_entry = -1; > - cfg->sections[curr_section] = malloc( > - sizeof(*cfg->sections[0]) + > - sizeof(cfg->sections[0]->entries[0]) * > - allocated_entries); > - if (cfg->sections[curr_section] == NULL) { > - printf("Error - no more memory\n"); > - goto error1; > - } > - > - snprintf(cfg->sections[curr_section]->name, > - sizeof(cfg->sections[0]->name), > - "%s", &buffer[1]); > + rte_cfgfile_add_section(cfg, &buffer[1]); > } else { > - /* value line */ > - if (curr_section < 0) { > - printf("Error line %d - value outside of" > - "section\n", lineno); > - goto error1; > - } > - > - struct rte_cfgfile_section *sect = > - cfg->sections[curr_section]; > - > + /* key and value line */ > char *split[2] = {NULL}; > + > split[0] = buffer; > split[1] = memchr(buffer, '=', len); > + *split[1] = '\0'; > + split[1]++; > + > + _strip(split[0], strlen(split[0])); > + _strip(split[1], strlen(split[1])); > > - /* when delimeter not found */ > - if (split[1] == NULL) { > + if (!(flags & CFG_FLAG_EMPTY_VALUES) && > + (*split[1] == '\0')) { > printf("Error at line %d - cannot " > "split string\n", lineno); This error message could do with an update. It's not that you can't split the string, it's that the value is empty - something not allowed. Also, it's bad practice to split literal strings for printing. This is the one case where you are encouraged to go over the 80-char limit rather than wrapping the line. /Bruce