[dpdk-dev] [PATCH v4 2/5] cfgfile: change existing API functions

[Bruce Richardson]

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 <jacekx.piasecki at intel.com>
> ---

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 <bruce.richardson at intel.com>

>  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 <stdlib.h>
>  #include <string.h>
>  #include <ctype.h>
> +#include <errno.h>
>  #include <rte_common.h>
>  
>  #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.

<snip>
> @@ -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.