Hello,
After previous set of patches have been committed I started working on
the last remaining piece - the merge functionality.
Re-reading the code I started some time ago I realized that at the time
I left the question of whether the config object should allow
partitioned section or not was still open.
Consider an example of the config file:
[foo]
x=1
[bar]
y=2
[foo]
z=3
Here we have an example of the case when the section is segmented or if
you look from another POW there are dup sections. So at first I thought
that it is OK to parse the file as is and have the internal config
object have two sections with same name. But after some deep thinking I
realized that it is a bad idea and creates a lot of unnecessary
complexity. So patch removes this code. This paves the way to a more
simple code in the merge function as we can assume that there are no dup
sections in the objects being merged.
I also noticed that doc build is spitting warnings so I opened a
separate ticket about it. That would be my next patch.
https://fedorahosted.org/sssd/ticket/1587
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From adfd4ec17878957423dba7592e3cd85369858b99 Mon Sep 17 00:00:00 2001
From: Dmitri Pal <d...@redhat.com>
Date: Wed, 17 Oct 2012 22:40:28 -0400
Subject: [PATCH] [INI] Remove code that allows dup sections
Duplicate sections in the config tree is a bad idea.
After more evaluation I decided to not allow more than one section
with the same name to be acceptable in the resulting object.
All duplicate sections need to be processed so that only
one section is left as a result. The collision flags control that.
It is OK to remove it as this code has not been released yet.
I also spotted that unit test does not exit if the system()
call was successful but the command it invoked was not.
Fixed that. Also fixed couple spelling mistakes here and there.
---
ini/ini.d/sexpect.conf | 170 ------------------------------------------------
ini/ini_configobj.h | 10 +--
ini/ini_fileobj.c | 1 -
ini/ini_parse.c | 10 +---
ini/ini_parse_ut.c | 40 +++++++----
5 files changed, 30 insertions(+), 201 deletions(-)
diff --git a/ini/ini.d/sexpect.conf b/ini/ini.d/sexpect.conf
index 6a9222b..93f7ed6 100644
--- a/ini/ini.d/sexpect.conf
+++ b/ini/ini.d/sexpect.conf
@@ -360,176 +360,6 @@ key1 = section2a Value 1
key2 = section2a Value 2
# Key 3
key3 = section2a Value 3
-# Section mode: ALLOW, value mode: OVERWRITE
-[section1]
-# Key 1
-key1 = section1a Value 1
-# Key 2
-key2 = section1a Value 2
-# Key 3
-key3 = section1a Value 3
-
-[section2]
-# Key 1
-key1 = section2a Value 1
-# Key 2
-key2 = section2a Value 2
-# Key 3
-key3 = section2a Value 3
-
-[section1]
-# Key 1
-key1 = section1b Value 1
-# Key 2
-key2 = section1b Value 2
-# Key 3
-key3 = section1b Value 3
-
-[section2]
-# Key 1
-key1 = section2b Value 1
-# Key 2
-key2 = section2b Value 2
-# Key 3
-key3 = section2b Value 3
-
-#End of file
-# Section mode: ALLOW, value mode: ERROR
-[section1]
-# Key 1
-key1 = section1a Value 1
-# Key 2
-key2 = section1a Value 2
-# Key 3
-key3 = section1a Value 3
-
-[section2]
-# Key 1
-key1 = section2a Value 1
-# Key 2
-key2 = section2a Value 2
-# Key 3
-key3 = section2a Value 3
-
-[section1]
-# Key 1
-key1 = section1b Value 1
-# Key 2
-key2 = section1b Value 2
-# Key 3
-key3 = section1b Value 3
-
-[section2]
-# Key 1
-key1 = section2b Value 1
-# Key 2
-key2 = section2b Value 2
-# Key 3
-key3 = section2b Value 3
-
-#End of file
-# Section mode: ALLOW, value mode: PRESERVE
-[section1]
-# Key 1
-key1 = section1a Value 1
-# Key 2
-key2 = section1a Value 2
-# Key 3
-key3 = section1a Value 3
-
-[section2]
-# Key 1
-key1 = section2a Value 1
-# Key 2
-key2 = section2a Value 2
-# Key 3
-key3 = section2a Value 3
-
-[section1]
-# Key 1
-key1 = section1b Value 1
-# Key 2
-key2 = section1b Value 2
-# Key 3
-key3 = section1b Value 3
-
-[section2]
-# Key 1
-key1 = section2b Value 1
-# Key 2
-key2 = section2b Value 2
-# Key 3
-key3 = section2b Value 3
-
-#End of file
-# Section mode: ALLOW, value mode: ALLOW
-[section1]
-# Key 1
-key1 = section1a Value 1
-# Key 2
-key2 = section1a Value 2
-# Key 3
-key3 = section1a Value 3
-
-[section2]
-# Key 1
-key1 = section2a Value 1
-# Key 2
-key2 = section2a Value 2
-# Key 3
-key3 = section2a Value 3
-
-[section1]
-# Key 1
-key1 = section1b Value 1
-# Key 2
-key2 = section1b Value 2
-# Key 3
-key3 = section1b Value 3
-
-[section2]
-# Key 1
-key1 = section2b Value 1
-# Key 2
-key2 = section2b Value 2
-# Key 3
-key3 = section2b Value 3
-
-#End of file
-# Section mode: ALLOW, value mode: DETECT
-[section1]
-# Key 1
-key1 = section1a Value 1
-# Key 2
-key2 = section1a Value 2
-# Key 3
-key3 = section1a Value 3
-
-[section2]
-# Key 1
-key1 = section2a Value 1
-# Key 2
-key2 = section2a Value 2
-# Key 3
-key3 = section2a Value 3
-
-[section1]
-# Key 1
-key1 = section1b Value 1
-# Key 2
-key2 = section1b Value 2
-# Key 3
-key3 = section1b Value 3
-
-[section2]
-# Key 1
-key1 = section2b Value 1
-# Key 2
-key2 = section2b Value 2
-# Key 3
-key3 = section2b Value 3
-
-#End of file
# Section mode: DETECT, value mode: OVERWRITE
[section1]
# Key 1
diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h
index 8385b8c..bf70528 100644
--- a/ini/ini_configobj.h
+++ b/ini/ini_configobj.h
@@ -278,7 +278,7 @@
* These flags should be used during parsing to handle duplicate
* keys coming from the same section scattered across the ini file.
* These flags also can be used to specify the rules of merging
- * values that come from two files separate configuration files.
+ * values that come from two different configuration files.
*
* @{
*/
@@ -304,7 +304,7 @@
* These flags should be used during parsing to handle duplicate
* sections scattered across the ini file.
* These flags also can be used to specify the rules of merging
- * sections that come from two separate configuration files.
+ * sections that come from two different configuration files.
*
* @{
*/
@@ -316,10 +316,8 @@
#define INI_MS_OVERWRITE 0x0200
/** @brief Second section is discarded */
#define INI_MS_PRESERVE 0x0300
-/** @brief Duplicates are allowed */
-#define INI_MS_ALLOW 0x0400
-/** @brief Duplicates are allowed but errors are logged */
-#define INI_MS_DETECT 0x0500
+/** @brief Merge but log errors if duplicate sections are detected */
+#define INI_MS_DETECT 0x0400
/**
* @}
diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c
index 73d7c4c..d26add4 100644
--- a/ini/ini_fileobj.c
+++ b/ini/ini_fileobj.c
@@ -65,7 +65,6 @@ int valid_collision_flags(uint32_t collision_flags)
(flag != INI_MS_OVERWRITE) &&
(flag != INI_MS_ERROR) &&
(flag != INI_MS_PRESERVE) &&
- (flag != INI_MS_ALLOW) &&
(flag != INI_MS_DETECT)) {
TRACE_ERROR_STRING("Invalid section collision flag","");
return 0;
diff --git a/ini/ini_parse.c b/ini/ini_parse.c
index 4491a5c..c61536d 100644
--- a/ini/ini_parse.c
+++ b/ini/ini_parse.c
@@ -500,7 +500,6 @@ static int parser_save_section(struct parser_obj *po)
{
int error = EOK;
uint32_t mergemode;
- int add = 0;
int merge = 0;
TRACE_FLOW_ENTRY();
@@ -550,11 +549,6 @@ static int parser_save_section(struct parser_obj *po)
po->sec = NULL;
break;
- case INI_MS_ALLOW:
- TRACE_INFO_STRING("Allow mode", "");
- add = 1;
- break;
-
case INI_MS_OVERWRITE:
/* Empty existing section */
TRACE_INFO_STRING("Ovewrite mode", "");
@@ -603,9 +597,7 @@ static int parser_save_section(struct parser_obj *po)
po->merge_sec = NULL;
}
- else add = 1;
-
- if (add) {
+ else {
/* Add section to configuration */
TRACE_INFO_STRING("Now adding collection", "");
error = col_add_collection_to_collection(po->top,
diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c
index bd90613..20b83af 100644
--- a/ini/ini_parse_ut.c
+++ b/ini/ini_parse_ut.c
@@ -226,8 +226,11 @@ int read_again_test(void)
error = system(command);
INIOUT(printf("Comparison of %s %s returned: %d\n",
infile, outfile, error));
- if (error) break;
-
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run copy command %d %d.\n", error, WEXITSTATUS(error));
+ error = -1;
+ break;
+ }
i++;
}
@@ -455,6 +458,10 @@ int merge_values_test(void)
error = system(command);
INIOUT(printf("Comparison of %s %s returned: %d\n",
resname, checkname, error));
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run copy command %d %d.\n", error, WEXITSTATUS(error));
+ return -1;
+ }
INIOUT(printf("<==== Merge values test end ====>\n"));
@@ -476,7 +483,6 @@ int merge_section_test(void)
INI_MS_ERROR,
INI_MS_OVERWRITE,
INI_MS_PRESERVE,
- INI_MS_ALLOW,
INI_MS_DETECT };
uint32_t mflags[] = { INI_MV2S_OVERWRITE,
@@ -489,7 +495,6 @@ int merge_section_test(void)
"ERROR",
"OVERWRITE",
"PRESERVE",
- "ALLOW",
"DETECT" };
const char *mstr[] = { "OVERWRITE",
@@ -524,7 +529,7 @@ int merge_section_test(void)
return error;
}
- for (i = 0; i < 6; i++) {
+ for (i = 0; i < 5; i++) {
for (j = 0; j < 5; j++) {
INIOUT(printf("<==== Testing mode %s + %s ====>\n",
@@ -656,6 +661,11 @@ int merge_section_test(void)
INIOUT(printf("Comparison of %s %s returned: %d\n",
resname, checkname, error));
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run diff command %d %d.\n", error, WEXITSTATUS(error));
+ return -1;
+ }
+
INIOUT(printf("<==== Merge section test end ====>\n"));
return error;
@@ -685,9 +695,9 @@ int startup_test(void)
INIOUT(printf("Running command '%s'\n", command));
error = system(command);
- if(error) {
- printf("Failed to run copy command %d.\n", error);
- return error;
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run copy command %d %d.\n", error, WEXITSTATUS(error));
+ return -1;
}
INIOUT(printf("Running chmod 660 on file '%s'\n", outfile));
@@ -804,14 +814,14 @@ int reload_test(void)
INIOUT(printf("Running command '%s'\n", command));
error = system(command);
- if(error) {
- printf("Failed to run copy command %d.\n", error);
- return error;
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run copy command %d %d.\n", error, WEXITSTATUS(error));
+ return -1;
}
INIOUT(printf("Running chmod 660 on file '%s'\n", outfile));
error = chmod(outfile, S_IRUSR | S_IWUSR);
- if(error) {
+ if (error) {
error = errno;
printf("Failed to run chmod command %d.\n", error);
return error;
@@ -912,10 +922,10 @@ int reload_test(void)
INIOUT(printf("Copy file again with command '%s'\n", command));
error = system(command);
- if(error) {
- printf("Failed to run copy command %d.\n", error);
+ if ((error) || (WEXITSTATUS(error))) {
+ printf("Failed to run copy command %d %d.\n", error, WEXITSTATUS(error));
ini_config_file_destroy(file_ctx);
- return error;
+ return -1;
}
INIOUT(printf("Read file again.\n"));
--
1.7.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
