On Tue, Jun 27, 2017 at 12:26:49PM +0200, Jacek Piasecki wrote:
> From: Kuba Kozak <kubax.ko...@intel.com>
> 
> 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 <jacekx.piase...@intel.com>
> ---
>  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

Reply via email to