[4/4] cfgfile: add unique name flag

Message ID 20240220035840.32978-5-fengchengwen@huawei.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series cfgfile: enhance error detecting |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS

Commit Message

fengchengwen Feb. 20, 2024, 3:58 a.m. UTC
  The cfgfile supports duplicate section name and entry name when parsing
config file, which may confused and hard to debug when accidentally set
duplicate name.

So add unique name flag, it will treat as error if encounter duplicate
section name or entry name.

Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cfgfile/rte_cfgfile.c | 32 +++++++++++++++++++++++---------
 lib/cfgfile/rte_cfgfile.h |  7 +++++++
 2 files changed, 30 insertions(+), 9 deletions(-)
  

Comments

Stephen Hemminger July 4, 2024, 9:36 p.m. UTC | #1
On Tue, 20 Feb 2024 03:58:40 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:

> The cfgfile supports duplicate section name and entry name when parsing
> config file, which may confused and hard to debug when accidentally set
> duplicate name.
> 
> So add unique name flag, it will treat as error if encounter duplicate
> section name or entry name.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

https://en.wikipedia.org/wiki/INI_file

	Interpretation of multiple section declarations with the same name also varies. 
	In some implementations, duplicate sections simply merge their properties, as if
	they occurred contiguously. Others may abort, or ignore some aspect of the INI file.

The standard reference for INI file parsing on Linux is the Python configparser
https://docs.python.org/3/library/configparser.html

	strict, default value: True

	When set to True, the parser will not allow for any section or option duplicates while reading from a single source 	(using read_file(), read_string() or read_dict()). It is recommended to use strict parsers in new applications.

The problem I see is that cfgfile allows duplicates on names, and sections.
Perhaps there should be a new strict flag for this.
  
fengchengwen July 5, 2024, 9:37 a.m. UTC | #2
Hi Stephen,

On 2024/7/5 5:36, Stephen Hemminger wrote:
> On Tue, 20 Feb 2024 03:58:40 +0000
> Chengwen Feng <fengchengwen@huawei.com> wrote:
> 
>> The cfgfile supports duplicate section name and entry name when parsing
>> config file, which may confused and hard to debug when accidentally set
>> duplicate name.
>>
>> So add unique name flag, it will treat as error if encounter duplicate
>> section name or entry name.
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> 
> https://en.wikipedia.org/wiki/INI_file
> 
> 	Interpretation of multiple section declarations with the same name also varies. 
> 	In some implementations, duplicate sections simply merge their properties, as if
> 	they occurred contiguously. Others may abort, or ignore some aspect of the INI file.
> 
> The standard reference for INI file parsing on Linux is the Python configparser
> https://docs.python.org/3/library/configparser.html
> 
> 	strict, default value: True
> 
> 	When set to True, the parser will not allow for any section or option duplicates while reading from a single source 	(using read_file(), read_string() or read_dict()). It is recommended to use strict parsers in new applications.
> 
> The problem I see is that cfgfile allows duplicates on names, and sections.
> Perhaps there should be a new strict flag for this.

It is important to keep the naming consistent, already sent v2 to fix it.

Thanks

> .
>
  

Patch

diff --git a/lib/cfgfile/rte_cfgfile.c b/lib/cfgfile/rte_cfgfile.c
index ad9314dc14..9e901b0977 100644
--- a/lib/cfgfile/rte_cfgfile.c
+++ b/lib/cfgfile/rte_cfgfile.c
@@ -102,8 +102,8 @@  _get_section(struct rte_cfgfile *cfg, const char *sectionname)
 }
 
 static int
-_add_entry(struct rte_cfgfile_section *section, const char *entryname,
-		const char *entryvalue)
+_add_entry(struct rte_cfgfile *cfg, struct rte_cfgfile_section *section,
+	   const char *entryname, const char *entryvalue, bool check_dup)
 {
 	int name_len, value_len;
 
@@ -115,6 +115,14 @@  _add_entry(struct rte_cfgfile_section *section, const char *entryname,
 		return -EINVAL;
 	}
 
+	if (check_dup) {
+		if (rte_cfgfile_has_entry(cfg, section->name, entryname) != 0) {
+			CFG_LOG(ERR, "duplicate entry name %s in section %s",
+				entryname, section->name);
+			return -EEXIST;
+		}
+	}
+
 	/* resize entry structure if we don't have room for more entries */
 	if (section->num_entries == section->allocated_entries) {
 		struct rte_cfgfile_entry *n_entries = realloc(
@@ -264,8 +272,9 @@  rte_cfgfile_load_with_params(const char *filename, int flags,
 			if (cfg->num_sections == 0)
 				goto error1;
 
-			ret = _add_entry(&cfg->sections[cfg->num_sections - 1],
-					 split[0], split[1]);
+			ret = _add_entry(cfg, &cfg->sections[cfg->num_sections - 1],
+					 split[0], split[1],
+					 !!(flags & CFG_FLAG_UNIQUE_NAME));
 			if (ret != 0)
 				goto error1;
 		}
@@ -286,7 +295,8 @@  rte_cfgfile_create(int flags)
 	struct rte_cfgfile *cfg;
 
 	/* future proof flags usage */
-	if (flags & ~(CFG_FLAG_GLOBAL_SECTION | CFG_FLAG_EMPTY_VALUES))
+	if (flags & ~(CFG_FLAG_GLOBAL_SECTION | CFG_FLAG_EMPTY_VALUES |
+		      CFG_FLAG_UNIQUE_NAME))
 		return NULL;
 
 	cfg = malloc(sizeof(*cfg));
@@ -356,6 +366,13 @@  rte_cfgfile_add_section(struct rte_cfgfile *cfg, const char *sectionname)
 		return -EINVAL;
 	}
 
+	if (cfg->flags & CFG_FLAG_UNIQUE_NAME) {
+		if (rte_cfgfile_has_section(cfg, sectionname) != 0) {
+			CFG_LOG(ERR, "duplicate section name %s", sectionname);
+			return -EEXIST;
+		}
+	}
+
 	/* resize overall struct if we don't have room for more	sections */
 	if (cfg->num_sections == cfg->allocated_sections) {
 
@@ -396,16 +413,13 @@  int rte_cfgfile_add_entry(struct rte_cfgfile *cfg,
 			|| (entryvalue == NULL))
 		return -EINVAL;
 
-	if (rte_cfgfile_has_entry(cfg, sectionname, entryname) != 0)
-		return -EEXIST;
-
 	/* search for section pointer by sectionname */
 	struct rte_cfgfile_section *curr_section = _get_section(cfg,
 								sectionname);
 	if (curr_section == NULL)
 		return -EINVAL;
 
-	ret = _add_entry(curr_section, entryname, entryvalue);
+	ret = _add_entry(cfg, curr_section, entryname, entryvalue, true);
 
 	return ret;
 }
diff --git a/lib/cfgfile/rte_cfgfile.h b/lib/cfgfile/rte_cfgfile.h
index 232c65c77b..74057c1b73 100644
--- a/lib/cfgfile/rte_cfgfile.h
+++ b/lib/cfgfile/rte_cfgfile.h
@@ -56,6 +56,13 @@  enum {
 	 * be zero length (e.g., "key=").
 	 */
 	CFG_FLAG_EMPTY_VALUES = 2,
+
+	/**
+	 * Indicates that file should have unique section name AND unique
+	 * entry's name. If a duplicate name is detected, the operation will
+	 * return error.
+	 */
+	CFG_FLAG_UNIQUE_NAME = 4,
 };
 /**@} */