diff mbox

[dpdk-dev] librte_cfgfile: add section num entries by index

Message ID 1493275900-4728-1-git-send-email-gprathyusha@caviumnetworks.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guduri Prathyusha April 27, 2017, 6:51 a.m. UTC 
  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>
---
 lib/librte_cfgfile/rte_cfgfile.c |   13 +++++++++++++
 lib/librte_cfgfile/rte_cfgfile.h |   20 ++++++++++++++++++++
 2 files changed, 33 insertions(+)

Comments

Thomas Monjalon July 31, 2017, 9:10 p.m. UTC | #1 
27/04/2017 08:51, Guduri Prathyusha:
> 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>
> ---
>  lib/librte_cfgfile/rte_cfgfile.c |   13 +++++++++++++
>  lib/librte_cfgfile/rte_cfgfile.h |   20 ++++++++++++++++++++
>  2 files changed, 33 insertions(+)

Cristian, this is an old patch not reviewed.
Any comment please?
Cristian Dumitrescu Aug. 1, 2017, 12:36 p.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, July 31, 2017 10:10 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Guduri Prathyusha
> <gprathyusha@caviumnetworks.com>
> Subject: Re: [dpdk-dev] [PATCH] librte_cfgfile: add section num entries by
> index
> 
> 27/04/2017 08:51, Guduri Prathyusha:
> > 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>
> > ---
> >  lib/librte_cfgfile/rte_cfgfile.c |   13 +++++++++++++
> >  lib/librte_cfgfile/rte_cfgfile.h |   20 ++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> 
> Cristian, this is an old patch not reviewed.
> Any comment please?

Do you need this reviewed for this release, or can it wait until the next release?

I have a few patches on my list to review (librte_cfgfile, librte_sched), I will make this review a priority for next release cycle.
Thomas Monjalon Aug. 1, 2017, 12:42 p.m. UTC | #3 
01/08/2017 14:36, Dumitrescu, Cristian:
> > 27/04/2017 08:51, Guduri Prathyusha:
> > > 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>
> > > ---
> > >  lib/librte_cfgfile/rte_cfgfile.c |   13 +++++++++++++
> > >  lib/librte_cfgfile/rte_cfgfile.h |   20 ++++++++++++++++++++
> > >  2 files changed, 33 insertions(+)
> > 
> > Cristian, this is an old patch not reviewed.
> > Any comment please?
> 
> Do you need this reviewed for this release, or can it wait until the next release?

It has already waited 3 months without any ping, so I guess it can wait.

> I have a few patches on my list to review (librte_cfgfile, librte_sched), I will make this review a priority for next release cycle.

OK thanks.
It's not about you specifically, but as a community, we need to improve
our response time (I'm not good as well).
Guduri Prathyusha Sept. 4, 2017, 6:12 a.m. UTC | #4 
ping???
Bruce Richardson Sept. 4, 2017, 10:58 a.m. UTC | #5 
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.

> +
> +	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
>
Guduri Prathyusha Sept. 4, 2017, 11:28 a.m. UTC | #6 
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
> >
Thomas Monjalon Sept. 15, 2017, 1:59 p.m. UTC | #7 
04/09/2017 12:58, Bruce Richardson:
> 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>

Applied, thanks
diff mbox

Patch

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);
+
+	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