Re: [PATCH] librte_cfgfile: add section num entries by index

[Guduri Prathyusha <gprathyusha@caviumnetworks.com>]

On Mon, Sep 04, 2017 at 11:58:44AM +0100, Bruce Richardson wrote:
> On Thu, Apr 27, 2017 at 12:21:40PM +0530, Guduri Prathyusha wrote:
> > rte_cfgfile_section_num_entries_by_index() is added to get the number of
> > entries of a section when multiple sections of the same name are
> > present.
> >
> > Signed-off-by: Guduri Prathyusha <gprathyusha@caviumnetworks.com>
>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/librte_cfgfile/rte_cfgfile.c |   13 +++++++++++++
> >  lib/librte_cfgfile/rte_cfgfile.h |   20 ++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
> > index b54a523..2588093 100644
> > --- a/lib/librte_cfgfile/rte_cfgfile.c
> > +++ b/lib/librte_cfgfile/rte_cfgfile.c
> > @@ -408,7 +408,20 @@ int rte_cfgfile_close(struct rte_cfgfile *cfg)
> >  	return s->num_entries;
> >  }
> >
> > +int
> > +rte_cfgfile_section_num_entries_by_index(struct rte_cfgfile *cfg,
> > +	char *sectionname, int index)
> > +{
> > +	const struct rte_cfgfile_section *sect;
> > +
> > +	if (index < 0 || index >= cfg->num_sections)
> > +		return -1;
> >
> > +	sect = cfg->sections[index];
> > +	snprintf(sectionname, CFG_NAME_LEN, "%s", sect->name);
>
> I don't like the fact that the section name is being overwritten here,
> but the behaviour is consistent with that done by the other "by_index()"
> functions, so it's probably better to keep this.
> In future a change to consider would be remove the sectionname entirely
> from the function arguments since it serves no purpose that I can see.
>
I agree that sectionname doesn't serve any purpose and can be removed. Thanks.
> > +
> > +	return sect->num_entries;
> > +}
> >  int
> >  rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const char *sectionname,
> >  		struct rte_cfgfile_entry *entries, int max_entries)
> > diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
> > index fa10d40..35dc419 100644
> > --- a/lib/librte_cfgfile/rte_cfgfile.h
> > +++ b/lib/librte_cfgfile/rte_cfgfile.h
> > @@ -184,6 +184,26 @@ int rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
> >  	const char *sectionname);
> >
> >  /**
> > +* Get number of entries in given config file section
> > +*
> > +* 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 sectionname
> > +*   Section name
> > +* @param index
> > +*   Section index
> > +* @return
> > +*   Number of entries in section on success, -1 otherwise
> > +*/
> > +int rte_cfgfile_section_num_entries_by_index(struct rte_cfgfile *cfg,
> > +	char *sectionname,
> > +	int index);
> > +
> > +/**
> >  * Get section entries as key-value pairs
> >  *
> >  * If multiple sections have the given name this function operates on the
> > --
> > 1.7.9.5
> >