From d0519d09d9bc3160ecef1f9ab1b3671b783f00c5 Mon Sep 17 00:00:00 2001
From: Dmitri Pal <dpal@redhat.com>
Date: Mon, 24 Dec 2012 11:31:24 -0500
Subject: [PATCH] [INI] Refactor interface a bit

After some evaluation it became apparent that error level
and collision flags should not be a part of the fileobj
but rather passed directly to the parser function.
This way the interface is more clean and logical.
This patch:
1) Removes the error level and collision flags from
the fileobj
2) Moves validation function from ini_fileobj.c
to ini_configobj.c where it belongs.
3) Moves error level and collision flags arguments
from file open function to the parser function.
4) The internals of the file obj are cleaned
5) Validation of the arguments is moved to parser function.
6) Unit tests are updated.
---
 ini/ini_config_priv.h |    4 ---
 ini/ini_configobj.c   |   41 ++++++++++++++++++++++++++++++++++++
 ini/ini_configobj.h   |   12 +++++-----
 ini/ini_fileobj.c     |   55 -------------------------------------------------
 ini/ini_parse.c       |   32 ++++++++++++++++++++++------
 ini/ini_parse_ut.c    |   37 ++++++++++++++++-----------------
 6 files changed, 90 insertions(+), 91 deletions(-)

diff --git a/ini/ini_config_priv.h b/ini/ini_config_priv.h
index 8b19d733afe9fb4b7ff38a5c0c2632e56c381225..ee6a4d7eda3fee645388bbfbdc17f121f67dd1a5 100644
--- a/ini/ini_config_priv.h
+++ b/ini/ini_config_priv.h
@@ -58,10 +58,6 @@ struct ini_cfgfile {
     char *filename;
     /* File stream */
     FILE *file;
-    /* Error level */
-    int error_level;
-    /* Collision flags - define how to merge things */
-    uint32_t collision_flags;
     /* What meta data to collect */
     uint32_t metadata_flags;
     /**********************/
diff --git a/ini/ini_configobj.c b/ini/ini_configobj.c
index 88b0279870a83c8af42c1b03e85f5f0d0b0200cf..4a3e96d2645f853a9f0b392fe9b3ffc206c11411 100644
--- a/ini/ini_configobj.c
+++ b/ini/ini_configobj.c
@@ -788,6 +788,47 @@ static int merge_configs(struct ini_cfgobj *donor,
     return error;
 }
 
+/* Check if collision flags are valid */
+int valid_collision_flags(uint32_t collision_flags)
+{
+    uint32_t flag;
+
+    TRACE_FLOW_ENTRY();
+
+    flag = collision_flags & INI_MV1S_MASK;
+    if ((flag != INI_MV1S_OVERWRITE) &&
+        (flag != INI_MV1S_ERROR) &&
+        (flag != INI_MV1S_PRESERVE) &&
+        (flag != INI_MV1S_ALLOW) &&
+        (flag != INI_MV1S_DETECT)) {
+        TRACE_ERROR_STRING("Invalid value collision flag","");
+        return 0;
+    }
+
+    flag = collision_flags & INI_MV2S_MASK;
+    if ((flag != INI_MV2S_OVERWRITE) &&
+        (flag != INI_MV2S_ERROR) &&
+        (flag != INI_MV2S_PRESERVE) &&
+        (flag != INI_MV2S_ALLOW) &&
+        (flag != INI_MV2S_DETECT)) {
+        TRACE_ERROR_STRING("Invalid value cross-section collision flag","");
+        return 0;
+    }
+
+    flag = collision_flags & INI_MS_MASK;
+    if ((flag != INI_MS_MERGE) &&
+        (flag != INI_MS_OVERWRITE) &&
+        (flag != INI_MS_ERROR) &&
+        (flag != INI_MS_PRESERVE) &&
+        (flag != INI_MS_DETECT)) {
+        TRACE_ERROR_STRING("Invalid section collision flag","");
+        return 0;
+    }
+
+    TRACE_FLOW_EXIT();
+    return 1;
+}
+
 /* Merge two configurations together creating a new one */
 int ini_config_merge(struct ini_cfgobj *first,
                      struct ini_cfgobj *second,
diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h
index 3572c2feaf68a061ec1ab117b0639366abb1f0a1..824df1797990584dc20e59b14cc99531d8f919f2 100644
--- a/ini/ini_configobj.h
+++ b/ini/ini_configobj.h
@@ -444,10 +444,6 @@ void ini_config_clean_state(struct ini_cfgobj *ini_config);
  *                              file name. If a short name is
  *                              specified the full path
  *                              will be resolved internally.
- * @param[in]  error_level      Flags that control actions
- *                              in case of parsing error.
- * @param[in]  collision_flags  Flags that control handling
- *                              of the duplicate sections or keys.
  * @param[in]  metadata_flags   Flags that specify what additional
  *                              data if any needs to be collected
  *                              about the ini file.
@@ -458,8 +454,6 @@ void ini_config_clean_state(struct ini_cfgobj *ini_config);
  * @return ENOMEM - No memory.
  */
 int ini_config_file_open(const char *filename,
-                         int error_level,
-                         uint32_t collision_flags,
                          uint32_t metadata_flags,
                          struct ini_cfgfile **file_ctx);
 
@@ -657,6 +651,10 @@ int ini_config_changed(struct ini_cfgfile *file_ctx1,
  * a parsing operation would fail with EINVAL.
  *
  * @param[in]  file_ctx         Configuration file object.
+ * @param[in]  error_level      Flags that control actions
+ *                              in case of parsing error.
+ * @param[in]  collision_flags  Flags that control handling
+ *                              of the duplicate sections or keys.
  * @param[out] ini_config       Configuration object.
  *
  * @return 0 - Success.
@@ -664,6 +662,8 @@ int ini_config_changed(struct ini_cfgfile *file_ctx1,
  * @return ENOMEM - No memory.
  */
 int ini_config_parse(struct ini_cfgfile *file_ctx,
+                     int error_level,
+                     uint32_t collision_flags,
                      struct ini_cfgobj *ini_config);
 
 /**
diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c
index c120180a50cd4048d393aad8b3290cfe15af749c..67734653e5b9534a4f177a2030ba317bbde76d78 100644
--- a/ini/ini_fileobj.c
+++ b/ini/ini_fileobj.c
@@ -32,48 +32,6 @@
 #include "collection_tools.h"
 
 
-/* Check if collision flags are valid */
-int valid_collision_flags(uint32_t collision_flags)
-{
-    uint32_t flag;
-
-    TRACE_FLOW_ENTRY();
-
-    flag = collision_flags & INI_MV1S_MASK;
-    if ((flag != INI_MV1S_OVERWRITE) &&
-        (flag != INI_MV1S_ERROR) &&
-        (flag != INI_MV1S_PRESERVE) &&
-        (flag != INI_MV1S_ALLOW) &&
-        (flag != INI_MV1S_DETECT)) {
-        TRACE_ERROR_STRING("Invalid value collision flag","");
-        return 0;
-    }
-
-    flag = collision_flags & INI_MV2S_MASK;
-    if ((flag != INI_MV2S_OVERWRITE) &&
-        (flag != INI_MV2S_ERROR) &&
-        (flag != INI_MV2S_PRESERVE) &&
-        (flag != INI_MV2S_ALLOW) &&
-        (flag != INI_MV2S_DETECT)) {
-        TRACE_ERROR_STRING("Invalid value cross-section collision flag","");
-        return 0;
-    }
-
-    flag = collision_flags & INI_MS_MASK;
-    if ((flag != INI_MS_MERGE) &&
-        (flag != INI_MS_OVERWRITE) &&
-        (flag != INI_MS_ERROR) &&
-        (flag != INI_MS_PRESERVE) &&
-        (flag != INI_MS_DETECT)) {
-        TRACE_ERROR_STRING("Invalid section collision flag","");
-        return 0;
-    }
-
-    TRACE_FLOW_EXIT();
-    return 1;
-}
-
-
 /* Close file but not destroy the object */
 void ini_config_file_close(struct ini_cfgfile *file_ctx)
 {
@@ -148,8 +106,6 @@ static int common_file_init(struct ini_cfgfile *file_ctx)
 
 /* Create a file object for parsing a config file */
 int ini_config_file_open(const char *filename,
-                         int error_level,
-                         uint32_t collision_flags,
                          uint32_t metadata_flags,
                          struct ini_cfgfile **file_ctx)
 {
@@ -163,11 +119,6 @@ int ini_config_file_open(const char *filename,
         return EINVAL;
     }
 
-    if (!valid_collision_flags(collision_flags)) {
-        TRACE_ERROR_NUMBER("Invalid flags.", EINVAL);
-        return EINVAL;
-    }
-
     /* Allocate structure */
     new_ctx = malloc(sizeof(struct ini_cfgfile));
     if (!new_ctx) {
@@ -180,8 +131,6 @@ int ini_config_file_open(const char *filename,
     new_ctx->error_list = NULL;
 
     /* Store flags */
-    new_ctx->error_level = error_level;
-    new_ctx->collision_flags = collision_flags;
     new_ctx->metadata_flags = metadata_flags;
     new_ctx->count = 0;
 
@@ -241,8 +190,6 @@ int ini_config_file_reopen(struct ini_cfgfile *file_ctx_in,
     new_ctx->error_list = NULL;
 
     /* Store flags */
-    new_ctx->error_level = file_ctx_in->error_level;
-    new_ctx->collision_flags = file_ctx_in->collision_flags;
     new_ctx->metadata_flags = file_ctx_in->metadata_flags;
     new_ctx->count = 0;
 
@@ -523,8 +470,6 @@ void ini_config_file_print(struct ini_cfgfile *file_ctx)
     else {
         printf("File name: %s\n", (file_ctx->filename) ? file_ctx->filename : "NULL");
         printf("File is %s\n", (file_ctx->file) ? "open" : "closed");
-        printf("Error level is %d\n", file_ctx->error_level);
-        printf("Collision flags %u\n", file_ctx->collision_flags);
         printf("Metadata flags %u\n", file_ctx->metadata_flags);
         if (file_ctx->error_list) col_print_collection(file_ctx->error_list);
         else printf("Error list is empty.");
diff --git a/ini/ini_parse.c b/ini/ini_parse.c
index 5690c2c2df75d82ed2bcedcbb8842973d53250ab..633cde8334be2817aec7445547d2f8bb1afb715e 100644
--- a/ini/ini_parse.c
+++ b/ini/ini_parse.c
@@ -1505,10 +1505,12 @@ int parser_run(struct parser_obj *po)
 
 /* Top level wrapper around the parser */
 int ini_config_parse(struct ini_cfgfile *file_ctx,
+                     int error_level,
+                     uint32_t collision_flags,
                      struct ini_cfgobj *ini_config)
 {
     int error = EOK;
-    struct parser_obj *po;
+    struct parser_obj *po = NULL;
     uint32_t fl1, fl2, fl3;
 
     TRACE_FLOW_ENTRY();
@@ -1518,11 +1520,27 @@ int ini_config_parse(struct ini_cfgfile *file_ctx,
         return EINVAL;
     }
 
+    if (!file_ctx) {
+        TRACE_ERROR_NUMBER("Invalid file context", EINVAL);
+        return EINVAL;
+    }
+
+    if (!valid_collision_flags(collision_flags)) {
+        TRACE_ERROR_NUMBER("Invalid flags.", EINVAL);
+        return EINVAL;
+    }
+
+    if ((error_level < INI_STOP_ON_ANY) || 
+        (error_level > INI_STOP_ON_ERROR)) {
+        /* Any other error mode is equivalent stopping on any error */
+        error_level = INI_STOP_ON_ANY;
+    }
+
     error = parser_create(ini_config,
                           file_ctx->file,
                           file_ctx->filename,
-                          file_ctx->error_level,
-                          file_ctx->collision_flags,
+                          error_level,
+                          collision_flags,
                           file_ctx->error_list,
                           &po);
     if (error) {
@@ -1532,9 +1550,9 @@ int ini_config_parse(struct ini_cfgfile *file_ctx,
 
     error = parser_run(po);
     if (error) {
-        fl1 = file_ctx->collision_flags & INI_MS_MASK;
-        fl2 = file_ctx->collision_flags & INI_MV1S_MASK;
-        fl3 = file_ctx->collision_flags & INI_MV2S_MASK;
+        fl1 = collision_flags & INI_MS_MASK;
+        fl2 = collision_flags & INI_MV1S_MASK;
+        fl3 = collision_flags & INI_MV2S_MASK;
         if ((error == EEXIST) &&
             (((fl1 == INI_MS_DETECT) &&
               (fl2 != INI_MV1S_ERROR) &&
@@ -1550,7 +1568,7 @@ int ini_config_parse(struct ini_cfgfile *file_ctx,
         }
         else {
             TRACE_ERROR_NUMBER("Failed to parse file", error);
-            TRACE_ERROR_NUMBER("Mode", file_ctx->collision_flags);
+            TRACE_ERROR_NUMBER("Mode", collision_flags);
             col_get_collection_count(file_ctx->error_list, &(file_ctx->count));
             if(file_ctx->count) (file_ctx->count)--;
             parser_destroy(po);
diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c
index f1e133c6f3ed48ed9a6c5074c6167373a5d6343b..2c77947c7c7e0ece66dfa62f25687a81c569814f 100644
--- a/ini/ini_parse_ut.c
+++ b/ini/ini_parse_ut.c
@@ -69,8 +69,6 @@ int test_one_file(const char *in_filename,
     }
 
     error = ini_config_file_open(in_filename,
-                                 INI_STOP_ON_NONE,
-                                 0, /* TBD */
                                  0, /* TBD */
                                  &file_ctx);
     if (error) {
@@ -81,6 +79,8 @@ int test_one_file(const char *in_filename,
     }
 
     error = ini_config_parse(file_ctx,
+                             INI_STOP_ON_NONE,
+                             0, /* TBD */
                              ini_config);
     if (error) {
         INIOUT(printf("Failed to parse configuration. Error %d.\n", error));
@@ -367,8 +367,6 @@ int merge_values_test(void)
 
         file_ctx = NULL;
         error = ini_config_file_open(filename,
-                                     INI_STOP_ON_ANY,
-                                     mflags[i],
                                      0, /* TBD */
                                      &file_ctx);
         if (error) {
@@ -382,6 +380,8 @@ int merge_values_test(void)
         }
 
         error = ini_config_parse(file_ctx,
+                                 INI_STOP_ON_ANY,
+                                 mflags[i],
                                  ini_config);
         if (error) {
             INIOUT(printf("Failed to parse configuration. Error %d.\n",
@@ -399,7 +399,7 @@ int merge_values_test(void)
                  (mflags[i]!= INI_MV1S_DETECT)) ||
                 ((mflags[i] = INI_MV1S_ERROR) && (error != EEXIST)) ||
                 ((mflags[i] = INI_MV1S_DETECT) && (error != EEXIST))) {
-                printf("This is unexpected error %d in mode %d\n", 
+                printf("This is unexpected error %d in mode %d\n",
                        error, mflags[i]);
                 ini_config_destroy(ini_config);
                 simplebuffer_free(sbobj);
@@ -562,8 +562,6 @@ int merge_section_test(void)
 
             file_ctx = NULL;
             error = ini_config_file_open(filename,
-                                         INI_STOP_ON_ANY,
-                                         msecflags[i] | mflags[j],
                                          0, /* TBD */
                                          &file_ctx);
             if (error) {
@@ -577,6 +575,8 @@ int merge_section_test(void)
             }
 
             error = ini_config_parse(file_ctx,
+                                     INI_STOP_ON_ANY,
+                                     msecflags[i] | mflags[j],
                                      ini_config);
             if (error) {
                 INIOUT(printf("Failed to parse configuration. "
@@ -685,8 +685,6 @@ int read_one_file(char *name,
 
     file_ctx = NULL;
     error = ini_config_file_open(name,
-                                 INI_STOP_ON_ANY,
-                                 collision_flags,
                                  0,
                                  &file_ctx);
     if (error) {
@@ -697,7 +695,10 @@ int read_one_file(char *name,
 
     INIOUT(printf("Parsing file %s\n", name));
 
-    error = ini_config_parse(file_ctx, ini_config);
+    error = ini_config_parse(file_ctx,
+                             INI_STOP_ON_ANY,
+                             collision_flags,
+                             ini_config);
     if (error) {
         INIOUT(printf("Failed to parse configuration. "
                       "Error %d.\n", error));
@@ -1082,8 +1083,6 @@ int startup_test(void)
 
     /* Open config file */
     error = ini_config_file_open(outfile,
-                                 INI_STOP_ON_NONE,
-                                 0,
                                  INI_META_STATS,
                                  &file_ctx);
     if (error) {
@@ -1136,6 +1135,8 @@ int startup_test(void)
     }
 
     error = ini_config_parse(file_ctx,
+                             INI_STOP_ON_NONE,
+                             0,
                              ini_config);
     if (error) {
         INIOUT(printf("Failed to parse configuration. Error %d.\n", error));
@@ -1204,8 +1205,6 @@ int reload_test(void)
 
     /* Open config file */
     error = ini_config_file_open(outfile,
-                                 INI_STOP_ON_NONE,
-                                 0,
                                  INI_META_STATS,
                                  &file_ctx);
     if (error) {
@@ -1407,11 +1406,6 @@ int get_test(void)
     INIOUT(printf("Reading file %s\n", infile));
 
     error = ini_config_file_open(infile,
-                                 INI_STOP_ON_NONE,
-                                 /* Merge section but allow duplicates */
-                                 INI_MS_MERGE |
-                                 INI_MV1S_ALLOW |
-                                 INI_MV2S_ALLOW,
                                  0,
                                  &file_ctx);
     if (error) {
@@ -1421,6 +1415,11 @@ int get_test(void)
     }
 
     error = ini_config_parse(file_ctx,
+                             INI_STOP_ON_NONE,
+                             /* Merge section but allow duplicates */
+                             INI_MS_MERGE |
+                             INI_MV1S_ALLOW |
+                             INI_MV2S_ALLOW,
                              ini_config);
     if (error) {
         INIOUT(printf("Failed to parse configuration. Error %d.\n", error));
-- 
1.7.1

