DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension
@ 2015-06-17 14:48 Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 1/6] librte_cfgfile: fix code formating in header file Maciej Gajdzica
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

Added new implementation of section parsing in config file. Refactored
existing code by spliting it to smaller functions. Changed section
allocation scheme and added new features  - variable length entry value
and line continue character '\'.

Pawel Wodkowski (6):
  librte_cfgfile: fix code formating in header file
  librte_compat: fix macro definition
  cfgfile: split rte_cfgfile_load to smaller functions
  cfgfile: added line continue character '\' to make multiline values  
      possible
  cfgfile: fixed calling free for each section in rte_cfgfile_close
  cfgfile: added new implementation of section parsing

 lib/librte_cfgfile/Makefile                |    2 +-
 lib/librte_cfgfile/rte_cfgfile.c           |  793 +++++++++++++++++++++-------
 lib/librte_cfgfile/rte_cfgfile.h           |   52 +-
 lib/librte_cfgfile/rte_cfgfile_version.map |    8 +
 lib/librte_compat/rte_compat.h             |    8 +-
 5 files changed, 653 insertions(+), 210 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 1/6] librte_cfgfile: fix code formating in header file
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 2/6] librte_compat: fix macro definition Maciej Gajdzica
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
 lib/librte_cfgfile/rte_cfgfile.h |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index 7c9fc91..860ce44 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -69,7 +69,8 @@ struct rte_cfgfile_entry {
 * @return
 *   Handle to configuration file
 */
-struct rte_cfgfile *rte_cfgfile_load(const char *filename, int flags);
+struct rte_cfgfile *
+rte_cfgfile_load(const char *filename, int flags);
 
 /**
 * Get number of sections in config file
@@ -83,7 +84,8 @@ struct rte_cfgfile *rte_cfgfile_load(const char *filename, int flags);
 * @return
 *   0 on success, error code otherwise
 */
-int rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sec_name,
+int
+rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sec_name,
 	size_t length);
 
 /**
@@ -103,7 +105,8 @@ int rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sec_name,
 * @return
 *   0 on success, error code otherwise
 */
-int rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
+int
+rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 	int max_sections);
 
 /**
@@ -116,7 +119,8 @@ int rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 * @return
 *   TRUE (value different than 0) if section exists, FALSE (value 0) otherwise
 */
-int rte_cfgfile_has_section(struct rte_cfgfile *cfg, const char *sectionname);
+int
+rte_cfgfile_has_section(struct rte_cfgfile *cfg, const char *sectionname);
 
 /**
 * Get number of entries in given config file section
@@ -128,7 +132,8 @@ int rte_cfgfile_has_section(struct rte_cfgfile *cfg, const char *sectionname);
 * @return
 *   Number of entries in section
 */
-int rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
+int
+rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 	const char *sectionname);
 
 /** Get section entries as key-value pairs
@@ -145,7 +150,8 @@ int rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 * @return
 *   0 on success, error code otherwise
 */
-int rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
+int
+rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
 	const char *sectionname,
 	struct rte_cfgfile_entry *entries,
 	int max_entries);
@@ -161,7 +167,8 @@ int rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
 * @return
 *   Entry value
 */
-const char *rte_cfgfile_get_entry(struct rte_cfgfile *cfg,
+const char *
+rte_cfgfile_get_entry(struct rte_cfgfile *cfg,
 	const char *sectionname,
 	const char *entryname);
 
@@ -176,7 +183,8 @@ const char *rte_cfgfile_get_entry(struct rte_cfgfile *cfg,
 * @return
 *   TRUE (value different than 0) if entry exists, FALSE (value 0) otherwise
 */
-int rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
+int
+rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
 	const char *entryname);
 
 /** Close config file
@@ -186,7 +194,8 @@ int rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
 * @return
 *   0 on success, error code otherwise
 */
-int rte_cfgfile_close(struct rte_cfgfile *cfg);
+int
+rte_cfgfile_close(struct rte_cfgfile *cfg);
 
 #ifdef __cplusplus
 }
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 2/6] librte_compat: fix macro definition
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 1/6] librte_cfgfile: fix code formating in header file Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 3/6] cfgfile: split rte_cfgfile_load to smaller functions Maciej Gajdzica
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
 lib/librte_compat/rte_compat.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_compat/rte_compat.h b/lib/librte_compat/rte_compat.h
index fb0dc19..cf6b9d3 100644
--- a/lib/librte_compat/rte_compat.h
+++ b/lib/librte_compat/rte_compat.h
@@ -63,7 +63,7 @@
  *	char foo(int value, int otherval) { ...}
  *
  * 5) Mark the newest version as the default version
- *	BIND_DEFAULT_SYMBOL(foo, 2.1);
+ *	BIND_DEFAULT_SYMBOL(foo, _v21, 2.1);
  *
  */
 
@@ -100,10 +100,10 @@
 /*
  * No symbol versioning in use
  */
-#define VERSION_SYMBOL(b, e, v)
+#define VERSION_SYMBOL(b, e, n)
 #define __vsym
-#define BASE_SYMBOL(b, n)
-#define BIND_DEFAULT_SYMBOL(b, v)
+#define BASE_SYMBOL(b, e)
+#define BIND_DEFAULT_SYMBOL(b, e, n)
 
 /*
  * RTE_BUILD_SHARED_LIB=n
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 3/6] cfgfile: split rte_cfgfile_load to smaller functions
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 1/6] librte_cfgfile: fix code formating in header file Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 2/6] librte_compat: fix macro definition Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 4/6] cfgfile: added line continue character '\' to make multiline values possible Maciej Gajdzica
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

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

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index b81c273..2e78583 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -37,6 +37,8 @@
 #include <ctype.h>
 #include <rte_string_fns.h>
 
+#include <rte_common.h>
+
 #include "rte_cfgfile.h"
 
 struct rte_cfgfile_section {
@@ -85,8 +87,67 @@ _strip(char *str, unsigned len)
 	return newlen;
 }
 
-struct rte_cfgfile *
-rte_cfgfile_load(const char *filename, int flags)
+static size_t
+strip_comment(char *buffer, size_t len)
+{
+	char *pos = memchr(buffer, ';', len);
+
+	if (pos == NULL)
+		return len;
+
+	if (len == 1) {
+		*pos = '\0';
+		return 0;
+	}
+
+	if (buffer[len - 1] == '\n') {
+		if (buffer[len - 2] == '\\') {
+			pos[0] = '\\';
+			pos[1] = '\n';
+			pos[2] = '\0';
+			len = pos - buffer + 2;
+		} else {
+			pos[0] = '\n';
+			pos[1] = '\0';
+			len = pos - buffer + 1;
+		}
+	}
+
+	return len;
+}
+
+/**
+ * Create new apty config file object.
+ *
+ * @param flags
+ *   Config file flags, Reserved for future use. Must be set to 0.
+ * @return
+ *   Handle to configuration file
+ */
+static struct rte_cfgfile *
+rte_cfgfile_create(__rte_unused int flags, int allocated_sections)
+{
+	struct rte_cfgfile *cfg = malloc(sizeof(*cfg) + sizeof(cfg->sections[0]) *
+		allocated_sections);
+
+	if (cfg != NULL)
+		memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections);
+
+	return cfg;
+}
+
+/**
+* Open config *file*.
+*
+* @param file
+*   Config stream to read.
+* @param flags
+*   Config file flags, Reserved for future use. Must be set to 0.
+* @return
+*   Handle to configuration file
+*/
+static struct rte_cfgfile *
+rte_cfgfile_read(FILE *f, int flags)
 {
 	int allocated_sections = CFG_ALLOC_SECTION_BATCH;
 	int allocated_entries = 0;
@@ -96,19 +157,14 @@ rte_cfgfile_load(const char *filename, int flags)
 	int lineno = 0;
 	struct rte_cfgfile *cfg = NULL;
 
-	FILE *f = fopen(filename, "r");
 	if (f == NULL)
 		return NULL;
 
-	cfg = malloc(sizeof(*cfg) + sizeof(cfg->sections[0]) *
-		allocated_sections);
+	cfg = rte_cfgfile_create(flags, allocated_sections);
 	if (cfg == NULL)
 		goto error2;
 
-	memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections);
-
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		char *pos = NULL;
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
@@ -116,11 +172,7 @@ rte_cfgfile_load(const char *filename, int flags)
 					"Check if line too long\n", lineno);
 			goto error1;
 		}
-		pos = memchr(buffer, ';', sizeof(buffer));
-		if (pos != NULL) {
-			*pos = '\0';
-			len = pos -  buffer;
-		}
+		len = strip_comment(buffer, len);
 
 		len = _strip(buffer, len);
 		if (buffer[0] != '[' && memchr(buffer, '=', len) == NULL)
@@ -238,6 +290,21 @@ error2:
 	return NULL;
 }
 
+struct rte_cfgfile *
+rte_cfgfile_load(const char *filename, int flags)
+{
+	struct rte_cfgfile *cfg = NULL;
+	FILE *file = fopen(filename, "r");
+
+	if (file == NULL)
+		return NULL;
+
+	cfg = rte_cfgfile_read(file, flags);
+	fclose(file);
+
+	return cfg;
+}
+
 
 int rte_cfgfile_close(struct rte_cfgfile *cfg)
 {
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 4/6] cfgfile: added line continue character '\' to make multiline values possible
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (2 preceding siblings ...)
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 3/6] cfgfile: split rte_cfgfile_load to smaller functions Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close Maciej Gajdzica
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

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

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 2e78583..4e77ef5 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -116,6 +116,122 @@ strip_comment(char *buffer, size_t len)
 	return len;
 }
 
+/* Get line from file. It concatanate lines with line continue.
+ * If *line is not NULL it must point to malloced data buffer and *len must
+ * be the length of *line.
+ */
+static int
+rte_cfgfile_getline(char **line, size_t *len, FILE *f, size_t *lineno)
+{
+	char *str = NULL;
+	size_t str_len = 0;
+
+	void *buf;
+	char *line_str = NULL;
+	size_t line_size = 0; /* Whole buffer size */
+	ssize_t line_len = 0; /* Length of line_str */
+
+	size_t line_cnt = 0;
+	int status = 0;
+
+	if (!line) {
+		status = EINVAL;
+		goto error_exit;
+	}
+
+	while (1) {
+		line_len = getline(&line_str, &line_size, f);
+		if (line_len <= 0) {
+			if (!feof(f)) {
+				status = -EIO;
+				goto error_exit;
+			}
+
+			break;
+		}
+
+		line_cnt++;
+		/* Replace last CR with LF */
+		if (line_len > 0 && line_str[line_len - 1] == '\r')
+			line_str[line_len - 1] = '\n';
+
+		/* Replace last CRLF with LF */
+		if (line_len > 1 && line_str[line_len - 2] == '\r' &&
+					line_str[line_len - 1] == '\n') {
+			line_str[line_len - 1] = '\0';
+			line_str[line_len - 2] = '\n';
+			line_len -= 1;
+		}
+
+		line_len = strip_comment(line_str, line_len);
+
+		/* Special case for first line. */
+		if (str == NULL) {
+			if (line_len == 0)
+				continue;
+
+			str = line_str;
+			str_len = line_len;
+
+			line_str = NULL;
+			line_len = 0;
+		} else {
+			if (line_len == 0)
+				break;
+
+			buf = realloc(str, str_len + line_len + 1);
+			if (buf == NULL) {
+				status = -ENOMEM;
+				goto error_exit;
+			}
+
+			str = buf;
+			memcpy(&str[str_len], line_str, line_len + 1);
+
+			str_len += line_len;
+		}
+
+		/* Check for line continue */
+		if (str_len < 2 || str[str_len - 1] != '\n' || str[str_len - 2] != '\\')
+			break;
+
+		/* Remove line continue character. */
+		str[str_len - 1] = '\0';
+		/* Change new line into space */
+		str[str_len - 2] = ' ';
+		str_len -= 1;
+	}
+
+	if (str_len) {
+		/* Squeeze str */
+		buf = realloc(str, str_len + 1);
+		if (buf == NULL) {
+			status = -ENOMEM;
+			goto error_exit;
+		}
+
+		free(*line);
+		*line = buf;
+	}
+
+	if (len)
+		*len = str_len;
+
+	if (lineno)
+		*lineno += line_cnt;
+
+	free(line_str);
+	return 0;
+
+error_exit:
+	printf("Error: %s\n", strerror(status));
+	if (lineno)
+		*lineno += line_cnt;
+	free(str);
+	free(line_str);
+	return status;
+}
+
 /**
  * Create new apty config file object.
  *
@@ -153,9 +269,11 @@ rte_cfgfile_read(FILE *f, int flags)
 	int allocated_entries = 0;
 	int curr_section = -1;
 	int curr_entry = -1;
-	char buffer[256];
-	int lineno = 0;
+	char *buffer = NULL;
+	size_t len;
+	size_t lineno = 0;
 	struct rte_cfgfile *cfg = NULL;
+	int status;
 
 	if (f == NULL)
 		return NULL;
@@ -164,15 +282,12 @@ rte_cfgfile_read(FILE *f, int flags)
 	if (cfg == NULL)
 		goto error2;
 
-	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		size_t len = strnlen(buffer, sizeof(buffer));
-		lineno++;
-		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
-			printf("Error line %d - no \\n found on string. "
-					"Check if line too long\n", lineno);
-			goto error1;
-		}
-		len = strip_comment(buffer, len);
+	while (!feof(f)) {
+		status = rte_cfgfile_getline(&buffer, &len, f, &lineno);
+		if (status)
+			break;
+		else if (!len)
+			continue;
 
 		len = _strip(buffer, len);
 		if (buffer[0] != '[' && memchr(buffer, '=', len) == NULL)
@@ -182,7 +297,7 @@ rte_cfgfile_read(FILE *f, int flags)
 			/* section heading line */
 			char *end = memchr(buffer, ']', len);
 			if (end == NULL) {
-				printf("Error line %d - no terminating '['"
+				printf("Error line %zu - no terminating '['"
 					"character found\n", lineno);
 				goto error1;
 			}
@@ -227,7 +342,7 @@ rte_cfgfile_read(FILE *f, int flags)
 		} else {
 			/* value line */
 			if (curr_section < 0) {
-				printf("Error line %d - value outside of"
+				printf("Error line %zu - value outside of"
 					"section\n", lineno);
 				goto error1;
 			}
@@ -237,7 +352,7 @@ rte_cfgfile_read(FILE *f, int flags)
 			char *split[2];
 			if (rte_strsplit(buffer, sizeof(buffer), split, 2, '=')
 				!= 2) {
-				printf("Error at line %d - cannot split "
+				printf("Error at line %zu - cannot split "
 					"string\n", lineno);
 				goto error1;
 			}
@@ -275,6 +390,10 @@ rte_cfgfile_read(FILE *f, int flags)
 				sizeof(entry->value)));
 		}
 	}
+
+	if (status)
+		goto error1;
+
 	fclose(f);
 	cfg->flags = flags;
 	cfg->num_sections = curr_section + 1;
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (3 preceding siblings ...)
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 4/6] cfgfile: added line continue character '\' to make multiline values possible Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-18 12:29   ` Thomas Monjalon
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing Maciej Gajdzica
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

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(-)

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;
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (4 preceding siblings ...)
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close Maciej Gajdzica
@ 2015-06-17 14:48 ` Maciej Gajdzica
  2015-06-18 12:37   ` Thomas Monjalon
  2015-06-17 16:51 ` [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Dumitrescu, Cristian
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Maciej Gajdzica @ 2015-06-17 14:48 UTC (permalink / raw)
  To: dev

From: Pawel Wodkowski <pawelx.wodkowski@intel.com>

New implementation of section parsing changes the way section allocation is
made. It protects against multiple sections with the same name and multiple
entries in section with the same name. It also drop constraint on value
length limited to 64 bytes. Now its size is unlimited. New features are part
of bigger modification and they are hard to be split to several smaller
patches.

Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
 lib/librte_cfgfile/Makefile                |    2 +-
 lib/librte_cfgfile/rte_cfgfile.c           |  550 +++++++++++++++++++---------
 lib/librte_cfgfile/rte_cfgfile.h           |   31 +-
 lib/librte_cfgfile/rte_cfgfile_version.map |    8 +
 4 files changed, 416 insertions(+), 175 deletions(-)

diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
index 032c240..616aef0 100644
--- a/lib/librte_cfgfile/Makefile
+++ b/lib/librte_cfgfile/Makefile
@@ -41,7 +41,7 @@ CFLAGS += $(WERROR_FLAGS)
 
 EXPORT_MAP := rte_cfgfile_version.map
 
-LIBABIVER := 1
+LIBABIVER := 2
 
 #
 # all source are stored in SRCS-y
diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 2ce5d70..a5867be 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -35,22 +35,26 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
-#include <rte_string_fns.h>
+#include <errno.h>
 
+#include <rte_compat.h>
 #include <rte_common.h>
+#include <rte_string_fns.h>
 
 #include "rte_cfgfile.h"
 
 struct rte_cfgfile_section {
 	char name[CFG_NAME_LEN];
 	size_t num_entries;
-	struct rte_cfgfile_entry *entries[0];
+	size_t allocated_entries;
+	struct rte_cfgfile_entry2 **entries;
 };
 
 struct rte_cfgfile {
 	int flags;
 	size_t num_sections;
-	struct rte_cfgfile_section *sections[0];
+	size_t allocated_sections;
+	struct rte_cfgfile_section **sections;
 };
 
 /** when we resize a file structure, how many extra entries
@@ -69,30 +73,47 @@ struct rte_cfgfile {
 	p;                          \
 })
 
-static unsigned
-_strip(char *str, unsigned len)
+static size_t
+strip_leading_spaces(char *str, size_t len)
 {
-	int newlen = len;
-	if (len == 0)
+	char *start = _skip_spaceses(str);
+
+	if (*start == '\0') {
+		str[0] = '\0';
 		return 0;
+	} else if (start == str)
+		return len;
 
-	if (isspace(str[len-1])) {
-		/* strip trailing whitespace */
-		while (newlen > 0 && isspace(str[newlen - 1]))
-			str[--newlen] = '\0';
-	}
+	len -= start - str;
+	memmove(str, start, len);
+	str[len] = '\0';
 
-	if (isspace(str[0])) {
-		/* strip leading whitespace */
-		int i, start = 1;
-		while (isspace(str[start]) && start < newlen)
-			start++
-			; /* do nothing */
-		newlen -= start;
-		for (i = 0; i < newlen; i++)
-			str[i] = str[i+start];
-		str[i] = '\0';
-	}
+	return len;
+}
+
+static size_t
+strip_trailing_spaces(char *str, size_t len)
+{
+	size_t newlen = len;
+
+	/* strip trailing whitespace */
+	while (newlen > 0 && isspace(str[newlen - 1]))
+		str[--newlen] = '\0';
+
+	return newlen;
+}
+
+static size_t
+strip_white_spaces(char *str, size_t len)
+{
+	size_t newlen;
+
+	if (len == 0)
+		return 0;
+
+	/* strip trailing whitespace */
+	newlen = strip_trailing_spaces(str, len);
+	newlen = strip_leading_spaces(str, len);
 	return newlen;
 }
 
@@ -241,6 +262,227 @@ error_exit:
 	return status;
 }
 
+/* Section functions */
+
+static struct rte_cfgfile_section *
+rte_cfgfile_find_section(struct rte_cfgfile *cfg, const char *sectionname)
+{
+	size_t i;
+	struct rte_cfgfile_section *sect;
+
+	for (i = 0; i < cfg->num_sections; i++) {
+		sect = cfg->sections[i];
+		if (strncmp(sect->name, sectionname, sizeof(sect->name)) == 0)
+			return sect;
+	}
+
+	return NULL;
+}
+
+static int
+rte_cfgfile_create_section(struct rte_cfgfile_section **section,
+		const char *name)
+{
+	struct rte_cfgfile_section *s = NULL;
+	size_t name_len;
+
+	if (name == NULL || section == NULL)
+		return -EINVAL;
+
+	name_len = strlen(name);
+
+	if (name_len == 0 || name_len >= sizeof(s->name) - 1) {
+		printf("Empty or too long section name\n");
+		return -EINVAL;
+	}
+
+	/* Create new section */
+	s = malloc(sizeof(struct rte_cfgfile_section));
+	if (s == NULL)
+		return -ENOMEM;
+
+	memcpy(s->name, name, name_len);
+	s->name[name_len] = '\0';
+	s->allocated_entries = 0;
+	s->num_entries = 0;
+	s->entries = NULL;
+
+	*section = s;
+	return 0;
+}
+
+static int
+rte_cfgfile_add_section(struct rte_cfgfile *cfg,
+		struct rte_cfgfile_section *new_section)
+{
+	struct rte_cfgfile_section *s;
+
+	if (cfg == NULL || new_section == NULL)
+		return -EINVAL;
+
+	s = rte_cfgfile_find_section(cfg, new_section->name);
+	if (s != NULL) {
+		printf("Duplicate section '%s'\n", new_section->name);
+		return -EEXIST;
+	}
+
+	if (cfg->num_sections == cfg->allocated_sections) {
+		size_t new_count = cfg->allocated_sections + CFG_ALLOC_SECTION_BATCH;
+		struct rte_cfgfile_section **entries = realloc(cfg->sections,
+				sizeof(struct rte_cfgfile_section *) * new_count);
+
+		if (entries == NULL)
+			return -ENOMEM;
+
+		cfg->allocated_sections = new_count;
+		cfg->sections = entries;
+	}
+
+	cfg->sections[cfg->num_sections] = new_section;
+	cfg->num_sections++;
+	return 0;
+}
+
+static int
+rte_cfgfile_parse_section(struct rte_cfgfile_section **s, const char *buffer)
+{
+	const char *start, *end, *garbage;
+	struct rte_cfgfile_section *new_section = NULL;
+	char name[CFG_NAME_LEN];
+	int status;
+	size_t name_len;
+
+	/* Check input arguments and buffer if it contain section */
+	if (s == NULL || buffer == NULL)
+		return -EINVAL;
+
+	/* Find start and end of name */
+	start = _skip_spaceses(buffer);
+	if (*start != '[')
+		return -EINVAL;
+
+	start = _skip_spaceses(start + 1);
+	end = strchr(start, ']');
+	if (end == NULL) {
+		printf("No section name delimiter ']' encoutered\n");
+		return -EINVAL;
+	}
+
+	garbage = _skip_spaceses(end + 1);
+	if (*garbage != '\0') {
+		printf("Garbage after section '%s' definition: %s\n",
+				buffer, garbage);
+		return -EINVAL;
+	}
+
+	/* Check if name is proper size */
+	if (start != end) {
+		for (end--; start != end; end--)
+			if (!isspace(*end))
+				break;
+		end++;
+	}
+
+	name_len = end - start;
+	if (name_len == 1 || name_len >= sizeof(new_section->name) - 1) {
+		printf("Empty or too long section name\n");
+		return -EINVAL;
+	}
+
+	memcpy(name, start, name_len);
+	name[name_len] = '\0';
+
+	status = rte_cfgfile_create_section(&new_section, name);
+	if (status != 0)
+		return status;
+
+	*s = new_section;
+	return 0;
+}
+
+/* Section entry functions */
+
+static inline struct rte_cfgfile_entry2 *
+rte_cfgfile_find_entry(struct rte_cfgfile_section *sect, const char *entryname)
+{
+	struct rte_cfgfile_entry2 *ret = NULL;
+	size_t i;
+
+	for (i = 0; i < sect->num_entries; i++) {
+		if (strcmp(sect->entries[i]->name, entryname) == 0) {
+			ret = sect->entries[i];
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int
+rte_cfgfile_section_add_entry(struct rte_cfgfile_section *section,
+		const char *name, const char *value)
+{
+	struct rte_cfgfile_entry2 *entry = NULL;
+	int status = 0;
+	size_t name_length, value_length;
+
+	if (section == NULL) {
+		status = -EINVAL;
+		goto error_exit;
+	}
+
+	entry = rte_cfgfile_find_entry(section, name);
+	if (entry != NULL) {
+		status = -EEXIST;
+		goto error_exit;
+	}
+
+	entry = malloc(sizeof(struct rte_cfgfile_entry2));
+	if (entry == NULL) {
+		status = ENOMEM;
+		goto error_exit;
+	}
+
+	name_length = snprintf(entry->name, sizeof(entry->name), "%s", name);
+	if (name_length >= RTE_DIM(entry->name)) {
+		status = EINVAL;
+		goto error_exit;
+	}
+
+	entry->value = strdup(value);
+	if (entry->value == NULL) {
+		status = ENOMEM;
+		goto error_exit;
+	}
+	value_length = strlen(entry->value);
+
+	strip_white_spaces(entry->name, name_length);
+	strip_white_spaces(entry->value, value_length);
+
+	/* Add entry */
+	if (section->num_entries == section->allocated_entries) {
+		size_t new_count = section->allocated_entries + CFG_ALLOC_ENTRY_BATCH;
+		struct rte_cfgfile_entry2 **new_entries = realloc(section->entries,
+				sizeof(section->entries[0]) * new_count);
+
+		if (new_entries == NULL) {
+			status = ENOMEM;
+			goto error_exit;
+		}
+		section->entries = new_entries;
+		section->allocated_entries = new_count;
+	}
+
+	section->entries[section->num_entries] = entry;
+	section->num_entries++;
+
+	return 0;
+error_exit:
+	printf("Error: %s\n", strerror(status));
+	free(entry);
+	return -status;
+}
+
 /**
  * Create new apty config file object.
  *
@@ -250,13 +492,12 @@ error_exit:
  *   Handle to configuration file
  */
 static struct rte_cfgfile *
-rte_cfgfile_create(__rte_unused int flags, int allocated_sections)
+rte_cfgfile_create(__rte_unused int flags)
 {
-	struct rte_cfgfile *cfg = malloc(sizeof(*cfg) + sizeof(cfg->sections[0]) *
-		allocated_sections);
+	struct rte_cfgfile *cfg = malloc(sizeof(struct rte_cfgfile));
 
 	if (cfg != NULL)
-		memset(cfg->sections, 0, sizeof(cfg->sections[0]) * allocated_sections);
+		memset(cfg, 0, sizeof(struct rte_cfgfile));
 
 	return cfg;
 }
@@ -274,154 +515,84 @@ rte_cfgfile_create(__rte_unused int flags, int allocated_sections)
 static struct rte_cfgfile *
 rte_cfgfile_read(FILE *f, int flags)
 {
-	int allocated_sections = CFG_ALLOC_SECTION_BATCH;
-	int allocated_entries = 0;
-	int curr_section = -1;
-	int curr_entry = -1;
-	char *buffer = NULL;
-	size_t len;
-	size_t lineno = 0;
+
+	char *line = NULL;
+	size_t line_size = 0;
+	char *pos;
+
 	struct rte_cfgfile *cfg = NULL;
-	int status;
+	struct rte_cfgfile_section *section = NULL;
+	size_t lineno = 0;
+	size_t current_lineno = 0;
+	int status = 0;
 
 	if (f == NULL)
 		return NULL;
 
-	cfg = rte_cfgfile_create(flags, allocated_sections);
+	cfg = rte_cfgfile_create(flags);
 	if (cfg == NULL)
 		goto error2;
 
 	while (!feof(f)) {
-		status = rte_cfgfile_getline(&buffer, &len, f, &lineno);
+		current_lineno = lineno;
+		status = rte_cfgfile_getline(&line, &line_size, f, &lineno);
 		if (status)
 			break;
-		else if (!len)
+		else if (!line_size)
 			continue;
 
-		len = _strip(buffer, len);
-		if (buffer[0] != '[' && memchr(buffer, '=', len) == NULL)
-			continue;
-
-		if (buffer[0] == '[') {
-			/* section heading line */
-			char *end = memchr(buffer, ']', len);
-			if (end == NULL) {
-				printf("Error line %zu - no terminating '['"
-					"character found\n", lineno);
-				goto error1;
-			}
-			*end = '\0';
-			_strip(&buffer[1], end - &buffer[1]);
-
-			/* close off old section and add start new one */
-			if (curr_section >= 0)
-				cfg->sections[curr_section]->num_entries =
-					curr_entry + 1;
-			curr_section++;
-
-			/* resize overall struct if we don't have room for more
-			sections */
-			if (curr_section == allocated_sections) {
-				allocated_sections += CFG_ALLOC_SECTION_BATCH;
-				struct rte_cfgfile *n_cfg = realloc(cfg,
-					sizeof(*cfg) + sizeof(cfg->sections[0])
-					* allocated_sections);
-				if (n_cfg == NULL) {
-					printf("Error - no more memory\n");
-					goto error1;
-				}
-				cfg = n_cfg;
-			}
-
-			/* allocate space for new section */
-			allocated_entries = CFG_ALLOC_ENTRY_BATCH;
-			curr_entry = -1;
-			cfg->sections[curr_section] = malloc(
-				sizeof(*cfg->sections[0]) +
-				sizeof(cfg->sections[0]->entries[0]) *
-				allocated_entries);
-			if (cfg->sections[curr_section] == NULL) {
-				printf("Error - no more memory\n");
-				goto error1;
-			}
-
-			snprintf(cfg->sections[curr_section]->name,
-					sizeof(cfg->sections[0]->name),
-					"%s", &buffer[1]);
-		} else {
-			/* value line */
-			if (curr_section < 0) {
-				printf("Error line %zu - value outside of"
-					"section\n", lineno);
-				goto error1;
+		pos = _skip_spaceses(line);
+		if (*pos == '[') {
+			status = rte_cfgfile_parse_section(&section, pos);
+			if (status == 0)
+				status = rte_cfgfile_add_section(cfg, section);
+		} else if (isalpha(*pos)) {
+			if (section == NULL) {
+				printf("Error line %zu - value outside of section\n",
+						current_lineno);
+				status = -EINVAL;
+				break;
 			}
 
-			struct rte_cfgfile_section *sect =
-				cfg->sections[curr_section];
 			char *split[2];
-			if (rte_strsplit(buffer, sizeof(buffer), split, 2, '=')
-				!= 2) {
-				printf("Error at line %zu - cannot split "
-					"string\n", lineno);
-				goto error1;
-			}
 
-			curr_entry++;
-			if (curr_entry == allocated_entries) {
-				allocated_entries += CFG_ALLOC_ENTRY_BATCH;
-				struct rte_cfgfile_section *n_sect = realloc(
-					sect, sizeof(*sect) +
-					sizeof(sect->entries[0]) *
-					allocated_entries);
-				if (n_sect == NULL) {
-					printf("Error - no more memory\n");
-					goto error1;
-				}
-				sect = cfg->sections[curr_section] = n_sect;
+			if (rte_strsplit(pos, line_size - (pos - line), split, 2, '=') == 2)
+				status = rte_cfgfile_section_add_entry(section,
+										split[0], split[1]);
+			else {
+				printf("Error at line %zu - cannot split string\n",
+						current_lineno);
+				status = -EINVAL;
 			}
+		} else if (*pos != '\0') {
+			printf("Invalid starting character '%c' on line %zu",
+					*pos, current_lineno);
 
-			sect->entries[curr_entry] = malloc(
-				sizeof(*sect->entries[0]));
-			if (sect->entries[curr_entry] == NULL) {
-				printf("Error - no more memory\n");
-				goto error1;
-			}
-
-			struct rte_cfgfile_entry *entry = sect->entries[
-				curr_entry];
-			snprintf(entry->name, sizeof(entry->name), "%s",
-				split[0]);
-			snprintf(entry->value, sizeof(entry->value), "%s",
-				split[1]);
-			_strip(entry->name, strnlen(entry->name,
-				sizeof(entry->name)));
-			_strip(entry->value, strnlen(entry->value,
-				sizeof(entry->value)));
+			status = -EINVAL;
 		}
+
+		if (status)
+			break;
 	}
 
 	if (status)
 		goto error1;
 
-	fclose(f);
+	free(line);
 	cfg->flags = flags;
-	cfg->num_sections = curr_section + 1;
-	/* curr_section will still be -1 if we have an empty file */
-	if (curr_section >= 0)
-		cfg->sections[curr_section]->num_entries = curr_entry + 1;
 	return cfg;
 
 error1:
 	rte_cfgfile_close(cfg);
 error2:
-	fclose(f);
+	free(line);
 	return NULL;
 }
 
 struct rte_cfgfile *
 rte_cfgfile_load(const char *filename, int flags)
 {
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 	FILE *file = fopen(filename, "r");
 
 	if (file == NULL)
@@ -460,10 +631,17 @@ rte_cfgfile_close(struct rte_cfgfile *cfg)
 
 int
 rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sectionname,
-size_t length)
+		size_t length)
 {
 	size_t i;
 	int num_sections = 0;
+
+	if (cfg == NULL || (length > 0 && sectionname == NULL))
+		return -EINVAL;
+
+	if (length == 0)
+		return cfg->num_sections;
+
 	for (i = 0; i < cfg->num_sections; i++) {
 		if (strncmp(cfg->sections[i]->name, sectionname, length) == 0)
 			num_sections++;
@@ -473,7 +651,7 @@ size_t length)
 
 int
 rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
-	int max_sections)
+		int max_sections)
 {
 	size_t i;
 
@@ -484,66 +662,100 @@ rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
 	return i;
 }
 
-static const struct rte_cfgfile_section *
-_get_section(struct rte_cfgfile *cfg, const char *sectionname)
-{
-	size_t i;
-	for (i = 0; i < cfg->num_sections; i++) {
-		if (strncmp(cfg->sections[i]->name, sectionname,
-				sizeof(cfg->sections[0]->name)) == 0)
-			return cfg->sections[i];
-	}
-	return NULL;
-}
-
 int
 rte_cfgfile_has_section(struct rte_cfgfile *cfg, const char *sectionname)
 {
-	return (_get_section(cfg, sectionname) != NULL);
+	return rte_cfgfile_find_section(cfg, sectionname) != NULL;
 }
 
 int
 rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 	const char *sectionname)
 {
-	const struct rte_cfgfile_section *s = _get_section(cfg, sectionname);
+	struct rte_cfgfile_section *s = rte_cfgfile_find_section(cfg, sectionname);
+
 	if (s == NULL)
 		return -1;
+
 	return s->num_entries;
 }
 
+int __vsym
+rte_cfgfile_section_entries_v20(struct rte_cfgfile *cfg,
+	const char *sectionname,
+	struct rte_cfgfile_entry *entries,
+	int max_entries)
+{
+	size_t i;
+	struct rte_cfgfile_section *sect;
 
-int
-rte_cfgfile_section_entries(struct rte_cfgfile *cfg, const char *sectionname,
-		struct rte_cfgfile_entry *entries, int max_entries)
+	if (max_entries < 0 || (max_entries > 0 && entries == NULL))
+		return -EINVAL;
+
+	sect = rte_cfgfile_find_section(cfg, sectionname);
+	if (sect == NULL)
+		return -ENOENT;
+
+	for (i = 0; (int)i < max_entries && i < sect->num_entries; i++) {
+		snprintf(entries[i].name, sizeof(entries[i].name), "%s",
+				sect->entries[i]->name);
+
+		snprintf(entries[i].value, sizeof(entries[i].value), "%s",
+						sect->entries[i]->name);
+	}
+
+	return i;
+}
+
+VERSION_SYMBOL(rte_cfgfile_section_entries, _v20, 2.0);
+
+int __vsym
+rte_cfgfile_section_entries_v21(struct rte_cfgfile *cfg,
+		const char *sectionname, const struct rte_cfgfile_entry2 **entries,
+		int max_entries)
 {
 	size_t i;
-	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
+	struct rte_cfgfile_section *sect;
+
+	if (max_entries < 0 || (max_entries > 0 && entries == NULL))
+		return -EINVAL;
+
+	sect = rte_cfgfile_find_section(cfg, sectionname);
 	if (sect == NULL)
-		return -1;
+		return -ENOENT;
+
 	for (i = 0; (int)i < max_entries && i < sect->num_entries; i++)
-		entries[i] = *sect->entries[i];
+		entries[i] = sect->entries[i];
+
 	return i;
 }
 
+BIND_DEFAULT_SYMBOL(rte_cfgfile_section_entries, _v21, 2.1);
+
 const char *
 rte_cfgfile_get_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname)
 {
-	size_t i;
-	const struct rte_cfgfile_section *sect = _get_section(cfg, sectionname);
+	struct rte_cfgfile_section *sect;
+	struct rte_cfgfile_entry2 *entry;
+
+	if (cfg == NULL)
+		return NULL;
+
+	sect = rte_cfgfile_find_section(cfg, sectionname);
 	if (sect == NULL)
 		return NULL;
-	for (i = 0; i < sect->num_entries; i++)
-		if (strncmp(sect->entries[i]->name, entryname, CFG_NAME_LEN)
-			== 0)
-			return sect->entries[i]->value;
-	return NULL;
+
+	entry = rte_cfgfile_find_entry(sect, entryname);
+	if (entry == NULL)
+		return NULL;
+
+	return entry->value;
 }
 
 int
 rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
 		const char *entryname)
 {
-	return (rte_cfgfile_get_entry(cfg, sectionname, entryname) != NULL);
+	return rte_cfgfile_get_entry(cfg, sectionname, entryname) != NULL;
 }
diff --git a/lib/librte_cfgfile/rte_cfgfile.h b/lib/librte_cfgfile/rte_cfgfile.h
index 860ce44..546b3b8 100644
--- a/lib/librte_cfgfile/rte_cfgfile.h
+++ b/lib/librte_cfgfile/rte_cfgfile.h
@@ -34,6 +34,8 @@
 #ifndef __INCLUDE_RTE_CFGFILE_H__
 #define __INCLUDE_RTE_CFGFILE_H__
 
+#include <rte_compat.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -59,6 +61,12 @@ struct rte_cfgfile_entry {
 	char value[CFG_VALUE_LEN]; /**< Value */
 };
 
+/** Configuration file entry */
+struct rte_cfgfile_entry2 {
+	char name[CFG_NAME_LEN]; /**< Name */
+	char *value; /**< Value */
+};
+
 /**
 * Open config file
 *
@@ -73,7 +81,9 @@ struct rte_cfgfile *
 rte_cfgfile_load(const char *filename, int flags);
 
 /**
-* Get number of sections in config file
+* Get number of sections in config file which name start with *length*
+* characters of *sec_name*. If *length* is 0 *sec_name* is ignored and total
+* sections count is returned.
 *
 * @param cfg
 *   Config file
@@ -82,7 +92,7 @@ rte_cfgfile_load(const char *filename, int flags);
 * @param length
 *   Maximum section name length
 * @return
-*   0 on success, error code otherwise
+*   Number of sections, or negative error code otherwise.
 */
 int
 rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sec_name,
@@ -103,7 +113,7 @@ rte_cfgfile_num_sections(struct rte_cfgfile *cfg, const char *sec_name,
 * @param max_sections
 *   Maximum number of section names to be stored in sections array
 * @return
-*   0 on success, error code otherwise
+*   Number of section names in cfg on success, error code otherwise
 */
 int
 rte_cfgfile_sections(struct rte_cfgfile *cfg, char *sections[],
@@ -150,12 +160,23 @@ rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg,
 * @return
 *   0 on success, error code otherwise
 */
-int
-rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
+int rte_cfgfile_section_entries(struct rte_cfgfile *cfg,
+	const char *sectionname,
+	struct rte_cfgfile_entry **entries,
+	int max_entries);
+
+int __vsym
+rte_cfgfile_section_entries_v20(struct rte_cfgfile *cfg,
 	const char *sectionname,
 	struct rte_cfgfile_entry *entries,
 	int max_entries);
 
+int __vsym
+rte_cfgfile_section_entries_v21(struct rte_cfgfile *cfg,
+	const char *sectionname,
+	const struct rte_cfgfile_entry2 **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..959756b 100644
--- a/lib/librte_cfgfile/rte_cfgfile_version.map
+++ b/lib/librte_cfgfile/rte_cfgfile_version.map
@@ -13,3 +13,11 @@ DPDK_2.0 {
 
 	local: *;
 };
+
+DPDK_2.1 {
+	global:
+
+	rte_cfgfile_section_entries;
+
+	local: *;
+}
\ No newline at end of file
-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (5 preceding siblings ...)
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing Maciej Gajdzica
@ 2015-06-17 16:51 ` Dumitrescu, Cristian
  2015-06-18 12:44 ` Thomas Monjalon
  2015-06-22 14:58 ` Gajdzica, MaciejX T
  8 siblings, 0 replies; 14+ messages in thread
From: Dumitrescu, Cristian @ 2015-06-17 16:51 UTC (permalink / raw)
  To: Gajdzica, MaciejX T, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Maciej Gajdzica
> Sent: Wednesday, June 17, 2015 3:49 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension
> 
> Added new implementation of section parsing in config file. Refactored
> existing code by spliting it to smaller functions. Changed section
> allocation scheme and added new features  - variable length entry value
> and line continue character '\'.
> 

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close Maciej Gajdzica
@ 2015-06-18 12:29   ` Thomas Monjalon
  2015-06-18 13:08     ` Gajdzica, MaciejX T
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-06-18 12:29 UTC (permalink / raw)
  To: Pawel Wodkowski; +Cc: dev

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing
  2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing Maciej Gajdzica
@ 2015-06-18 12:37   ` Thomas Monjalon
  2015-06-18 13:09     ` Gajdzica, MaciejX T
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2015-06-18 12:37 UTC (permalink / raw)
  To: Pawel Wodkowski; +Cc: dev

2015-06-17 16:48, Maciej Gajdzica:
> +       char *start = _skip_spaceses(str);

Why not use a function with "char *" parameter instead of a macro?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (6 preceding siblings ...)
  2015-06-17 16:51 ` [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Dumitrescu, Cristian
@ 2015-06-18 12:44 ` Thomas Monjalon
  2015-06-22 14:58 ` Gajdzica, MaciejX T
  8 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2015-06-18 12:44 UTC (permalink / raw)
  To: Pawel Wodkowski; +Cc: dev

2015-06-17 16:48, Maciej Gajdzica:
> Added new implementation of section parsing in config file. Refactored
> existing code by spliting it to smaller functions. Changed section
> allocation scheme and added new features  - variable length entry value
> and line continue character '\'.
> 
> Pawel Wodkowski (6):
>   librte_cfgfile: fix code formating in header file
>   librte_compat: fix macro definition
>   cfgfile: split rte_cfgfile_load to smaller functions
>   cfgfile: added line continue character '\' to make multiline values  
>       possible
>   cfgfile: fixed calling free for each section in rte_cfgfile_close
>   cfgfile: added new implementation of section parsing

The shared library cannot link:
	syntax error in VERSION script

>  lib/librte_cfgfile/Makefile                |    2 +-
>  lib/librte_cfgfile/rte_cfgfile.c           |  793 +++++++++++++++++++++-------
>  lib/librte_cfgfile/rte_cfgfile.h           |   52 +-
>  lib/librte_cfgfile/rte_cfgfile_version.map |    8 +
>  lib/librte_compat/rte_compat.h             |    8 +-
>  5 files changed, 653 insertions(+), 210 deletions(-)

You forgot to update the example qos_sched (as previously noted):
	examples/qos_sched/cfg_file.c:485:47: error:
	passing argument 3 of ‘rte_cfgfile_section_entries’ from incompatible pointer type

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close
  2015-06-18 12:29   ` Thomas Monjalon
@ 2015-06-18 13:08     ` Gajdzica, MaciejX T
  0 siblings, 0 replies; 14+ messages in thread
From: Gajdzica, MaciejX T @ 2015-06-18 13:08 UTC (permalink / raw)
  To: Thomas Monjalon, Wodkowski, PawelX; +Cc: dev

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing
  2015-06-18 12:37   ` Thomas Monjalon
@ 2015-06-18 13:09     ` Gajdzica, MaciejX T
  0 siblings, 0 replies; 14+ messages in thread
From: Gajdzica, MaciejX T @ 2015-06-18 13:09 UTC (permalink / raw)
  To: Thomas Monjalon, Wodkowski, PawelX; +Cc: dev

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 18, 2015 2:37 PM
> To: Wodkowski, PawelX
> Cc: dev@dpdk.org; Gajdzica, MaciejX T
> Subject: Re: [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of
> section parsing
> 
> 2015-06-17 16:48, Maciej Gajdzica:
> > +       char *start = _skip_spaceses(str);
> 
> Why not use a function with "char *" parameter instead of a macro?

I agree, function will be better than macro, will fix that in next version

Best Regards
Maciek

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension
  2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
                   ` (7 preceding siblings ...)
  2015-06-18 12:44 ` Thomas Monjalon
@ 2015-06-22 14:58 ` Gajdzica, MaciejX T
  8 siblings, 0 replies; 14+ messages in thread
From: Gajdzica, MaciejX T @ 2015-06-22 14:58 UTC (permalink / raw)
  To: dev

> -----Original Message-----
> From: Gajdzica, MaciejX T
> Sent: Wednesday, June 17, 2015 4:49 PM
> To: dev@dpdk.org
> Cc: Gajdzica, MaciejX T
> Subject: [PATCH v2 0/6] cfgfile: config file parsing extension
> 
> Added new implementation of section parsing in config file. Refactored existing
> code by spliting it to smaller functions. Changed section allocation scheme and
> added new features  - variable length entry value and line continue character '\'.
> 
> Pawel Wodkowski (6):
>   librte_cfgfile: fix code formating in header file
>   librte_compat: fix macro definition
>   cfgfile: split rte_cfgfile_load to smaller functions
>   cfgfile: added line continue character '\' to make multiline values
>       possible
>   cfgfile: fixed calling free for each section in rte_cfgfile_close
>   cfgfile: added new implementation of section parsing
> 
>  lib/librte_cfgfile/Makefile                |    2 +-
>  lib/librte_cfgfile/rte_cfgfile.c           |  793 +++++++++++++++++++++-------
>  lib/librte_cfgfile/rte_cfgfile.h           |   52 +-
>  lib/librte_cfgfile/rte_cfgfile_version.map |    8 +
>  lib/librte_compat/rte_compat.h             |    8 +-
>  5 files changed, 653 insertions(+), 210 deletions(-)
> 
> --
> 1.7.9.5

NACK - After reviewing this patchset, we decided that those changes are not acceptable in current form.
New section allocation scheme doesn't add much value. Also new implementation of rte_cfgfile_entry with
static string for name and dynamic for value may cause confusion. We will go back to cfgfile improvements in 2.2.

Best Regards
Maciek

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-06-22 14:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 14:48 [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Maciej Gajdzica
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 1/6] librte_cfgfile: fix code formating in header file Maciej Gajdzica
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 2/6] librte_compat: fix macro definition Maciej Gajdzica
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 3/6] cfgfile: split rte_cfgfile_load to smaller functions Maciej Gajdzica
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 4/6] cfgfile: added line continue character '\' to make multiline values possible Maciej Gajdzica
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 5/6] cfgfile: fixed calling free for each section in rte_cfgfile_close Maciej Gajdzica
2015-06-18 12:29   ` Thomas Monjalon
2015-06-18 13:08     ` Gajdzica, MaciejX T
2015-06-17 14:48 ` [dpdk-dev] [PATCH v2 6/6] cfgfile: added new implementation of section parsing Maciej Gajdzica
2015-06-18 12:37   ` Thomas Monjalon
2015-06-18 13:09     ` Gajdzica, MaciejX T
2015-06-17 16:51 ` [dpdk-dev] [PATCH v2 0/6] cfgfile: config file parsing extension Dumitrescu, Cristian
2015-06-18 12:44 ` Thomas Monjalon
2015-06-22 14:58 ` Gajdzica, MaciejX T

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).