diff mbox

[dpdk-dev,v2,5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close

Message ID 1434552528-3576-6-git-send-email-maciejx.t.gajdzica@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Maciej Gajdzica June 17, 2015, 2:48 p.m. UTC 
  From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
 lib/librte_cfgfile/rte_cfgfile.c |   53 +++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 23 deletions(-)

Comments

Thomas Monjalon June 18, 2015, 12:29 p.m. UTC | #1 
2015-06-17 16:48, Maciej Gajdzica:
> From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> 
> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>

What is fixed exactly? What was the problem?

> @@ -60,6 +60,15 @@ struct rte_cfgfile {
>   * for new entries do we add in */
>  #define CFG_ALLOC_ENTRY_BATCH 16
>  
> +/* Helpers */
> +
> +#define _skip_spaceses(str) ({  \
> +	__typeof__(str) p = (str);  \
> +	while (isspace(*p))         \
> +		p++;                    \
> +	p;                          \
> +})

This macro is not used in this patch nor related.
Is "spaceses" a typo?

> @@ -523,7 +530,7 @@ const char *
>  rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
>  		const char *entryname)
>  {
> -	int i;
> +	size_t i;

Why this change? seems not related to free.
Maciej Gajdzica June 18, 2015, 1:08 p.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 18, 2015 2:30 PM
> To: Wodkowski, PawelX
> Cc: dev@dpdk.org; Gajdzica, MaciejX T
> Subject: Re: [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section
> in rte_cfgfile_close
> 
> 2015-06-17 16:48, Maciej Gajdzica:
> > From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> >
> > Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> 
> What is fixed exactly? What was the problem?

I will merge this patch with 6th patch in the series. Freeing memory is changed because structure
that is freed changed and this change belongs to next patch in the series.

> 
> > @@ -60,6 +60,15 @@ struct rte_cfgfile {
> >   * for new entries do we add in */
> >  #define CFG_ALLOC_ENTRY_BATCH 16
> >
> > +/* Helpers */
> > +
> > +#define _skip_spaceses(str) ({  \
> > +	__typeof__(str) p = (str);  \
> > +	while (isspace(*p))         \
> > +		p++;                    \
> > +	p;                          \
> > +})
> 
> This macro is not used in this patch nor related.
> Is "spaceses" a typo?

Yes it's a typo, thanks.

> 
> > @@ -523,7 +530,7 @@ const char *
> >  rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
> >  		const char *entryname)
> >  {
> > -	int i;
> > +	size_t i;
> 
> Why this change? seems not related to free.
> 
num_entries and num_sections variables changed type from int to size_t and now comparison with
i produces warning when compiling.
diff mbox

Patch

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 4e77ef5..2ce5d70 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -43,13 +43,13 @@ 
 
 struct rte_cfgfile_section {
 	char name[CFG_NAME_LEN];
-	int num_entries;
+	size_t num_entries;
 	struct rte_cfgfile_entry *entries[0];
 };
 
 struct rte_cfgfile {
 	int flags;
-	int num_sections;
+	size_t num_sections;
 	struct rte_cfgfile_section *sections[0];
 };
 
@@ -60,6 +60,15 @@  struct rte_cfgfile {
  * for new entries do we add in */
 #define CFG_ALLOC_ENTRY_BATCH 16
 
+/* Helpers */
+
+#define _skip_spaceses(str) ({  \
+	__typeof__(str) p = (str);  \
+	while (isspace(*p))         \
+		p++;                    \
+	p;                          \
+})
+
 static unsigned
 _strip(char *str, unsigned len)
 {
@@ -424,28 +433,26 @@  rte_cfgfile_load(const char *filename, int flags)
 	return cfg;
 }
 
-
-int rte_cfgfile_close(struct rte_cfgfile *cfg)
+int
+rte_cfgfile_close(struct rte_cfgfile *cfg)
 {
-	int i, j;
+	struct rte_cfgfile_section *s;
+	size_t i, j;
 
 	if (cfg == NULL)
 		return -1;
 
 	for (i = 0; i < cfg->num_sections; i++) {
-		if (cfg->sections[i] != NULL) {
-			if (cfg->sections[i]->num_entries) {
-				for (j = 0; j < cfg->sections[i]->num_entries;
-					j++) {
-					if (cfg->sections[i]->entries[j] !=
-						NULL)
-						free(cfg->sections[i]->
-							entries[j]);
-				}
-			}
-			free(cfg->sections[i]);
+		s = cfg->sections[i];
+		for (j = 0; j < s->num_entries; j++) {
+			free(cfg->sections[i]->entries[j]->value);
+			free(cfg->sections[i]->entries[j]);
 		}
+		free(cfg->sections[i]->entries);
+		free(s);
 	}
+
+	free(cfg->sections);
 	free(cfg);
 
 	return 0;
@@ -455,7 +462,7 @@  int
 rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sectionname,
 size_t length)
 {
-	int i;
+	size_t i;
 	int num_sections = 0;
 	for (i = 0; i < cfg->num_sections; i++) {
 		if (strncmp(cfg->sections[i]->name, sectionname, length) == 0)
@@ -468,9 +475,9 @@  int
 rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 	int max_sections)
 {
-	int i;
+	size_t i;
 
-	for (i = 0; i < cfg->num_sections && i < max_sections; i++)
+	for (i = 0; i < cfg->num_sections && (int)i < max_sections; i++)
 		snprintf(sections[i], CFG_NAME_LEN, "%s",
 		cfg->sections[i]->name);
 
@@ -480,7 +487,7 @@  rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 static const struct rte_cfgfile_section *
 _get_section(struct rte_cfgfile *cfg, const char *sectionname)
 {
-	int i;
+	size_t i;
 	for (i = 0; i < cfg->num_sections; i++) {
 		if (strncmp(cfg->sections[i]->name, sectionname,
 				sizeof(cfg->sections[0]->name)) == 0)
@@ -510,11 +517,11 @@  int
 rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const char *sectionname,
 		struct rte_cfgfile_entry *entries, int max_entries)
 {
-	int i;
+	size_t i;
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return -1;
-	for (i = 0; i < max_entries && i < sect->num_entries; i++)
+	for (i = 0; (int)i < max_entries && i < sect->num_entries; i++)
 		entries[i] = *sect->entries[i];
 	return i;
 }
@@ -523,7 +530,7 @@  const char *
 rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname)
 {
-	int i;
+	size_t i;
 	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
 	if (sect == NULL)
 		return NULL;