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.

6 files changed, 89 insertions, 90 deletions

diff --git a/ini/ini_config_priv.h b/ini/ini_config_priv.h
index 8b19d73..ee6a4d7 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 88b0279..4a3e96d 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 3572c2f..824df17 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 c120180..6773465 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 5690c2c..2f65dfa 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 d6f812f..2c77947 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",
@@ -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));