Re: [dpdk-dev] [PATCH v2] cfgfile: support looking up sections by index

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Rich Lane [mailto:rich.lane@bigswitch.com]
> Sent: Tuesday, January 19, 2016 4:42 AM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Panu Matilainen
> <pmatilai@redhat.com>
> Subject: [PATCH v2] cfgfile: support looking up sections by index
> 
> This is useful when sections have duplicate names.

Hi Rich,

You are right, I can see this is needed to allow multiple sections with identical name in the same configuration file. When sections with the same name are not allowed, then this is not needed, as the current API is sufficient.

To me, having several sections with the same name does not look like a good idea, in fact it might look like an application design flaw, as differentiating between sections with the same name can only done based on the position of the section in the configuration file, which is an error prone option. Some examples: 
1. While maintaining a large config file, keeping a specific section at a fixed position quickly becomes a pain, and shifting the position of the section up or down can completely change the application behavior;
2. Using basic pre-processors (like CPP or M4) or scripts, the master configuration file can include other configuration files, with the inclusion of each decided at run-time based on application command line parameters, so the position of certain sections is actually not known until run-time.

Can you provide some examples when having multiple sections with the same name is a key requirement?

A straight forward workaround to having multiple sections with the same name is to add a number to the name of each such section. Using the current API, all the sections with the same prefix name can be read gracefully. As an example, the ip_pipeline application allows multiple sections with the same name prefix but a different number prefix:
PIPELINE0, PIPELINE1, ...
LINK0, LINK1, ...
MEMPOOL0, MEMPOOL1, ...
RXQ0.0, RXQ0.1, RXQ1.0, ...
TXQ0.0, TXQ0.1, TXQ1.0, ...

Is there a reason why this approach is not acceptable for your application?

Regards,
Cristian

> 
> Signed-off-by: Rich Lane <rlane@bigswitch.com>
> ---
> v1->v2:
> - Added new symbol to version script.
> 
>  lib/librte_cfgfile/rte_cfgfile.c           | 16 ++++++++++++++++
>  lib/librte_cfgfile/rte_cfgfile.h           | 23 +++++++++++++++++++++++
>  lib/librte_cfgfile/rte_cfgfile_version.map |  6 ++++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
> index a677dad..0bb40a4 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -333,6 +333,22 @@ rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
> const char *sectionname,
>  	return i;
>  }
> 
> +int
> +rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg, int index,
> +		struct rte_cfgfile_entry *entries, int max_entries)
> +{
> +	int i;
> +	const struct rte_cfgfile_section *sect;
> +
> +	if (index >= cfg->num_sections)
> +		return -1;
> +
> +	sect = cfg->sections[index];
> +	for (i = 0; i < max_entries && i < sect->num_entries; i++)
> +		entries[i] = *sect->entries[i];
> +	return i;
> +}
> +
>  const char *
>  rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
>  		const char *entryname)
> diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
> index d443782..8e69971 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.h
> +++ b/lib/librte_cfgfile/rte_cfgfile.h
> @@ -155,6 +155,29 @@ int rte_cfgfile_section_entries(struct rte_cfgfile
> *cfg,
>  	struct rte_cfgfile_entry *entries,
>  	int max_entries);
> 
> +/** Get section entries as key-value pairs
> +*
> +* The index of a section is the same as the index of its name in the
> +* result of rte_cfgfile_sections. This API can be used when there are
> +* multiple sections with the same name.
> +*
> +* @param cfg
> +*   Config file
> +* @param index
> +*   Section index
> +* @param entries
> +*   Pre-allocated array of at least max_entries entries where the section
> +*   entries are stored as key-value pair after successful invocation
> +* @param max_entries
> +*   Maximum number of section entries to be stored in entries array
> +* @return
> +*   Number of entries populated on success, negative error code otherwise
> +*/
> +int rte_cfgfile_section_entries_by_index(struct rte_cfgfile *cfg,
> +	int index,
> +	struct rte_cfgfile_entry *entries,
> +	int max_entries);
> +
>  /** Get value of the named entry in named config file section
>  *
>  * @param cfg
> diff --git a/lib/librte_cfgfile/rte_cfgfile_version.map
> b/lib/librte_cfgfile/rte_cfgfile_version.map
> index bf6c6fd..f6a27a9 100644
> --- a/lib/librte_cfgfile/rte_cfgfile_version.map
> +++ b/lib/librte_cfgfile/rte_cfgfile_version.map
> @@ -13,3 +13,9 @@ DPDK_2.0 {
> 
>  	local: *;
>  };
> +
> +DPDK_2.3 {
> +	global:
> +
> +	rte_cfgfile_section_entries_by_index;
> +} DPDK_2.0;
> --
> 1.9.1