Fix bug 99.

- Handle the case where the parent node already exists in ConfSet.
- Deal with allow_override properly when a node has already been
  set with ConfSet.

src/conf-yaml-loader.c

@@ -84,32 +84,28 @@ ConfYamlParse(yaml_parser_t *parser, ConfNode *parent, int inseq)
             }
             else {
                 if (state == CONF_KEY) {
-                    /* If the node already exists, check if we can
-                     * override it.  If we can, free it then continue
-                     * otherwise move onto the next configuration
-                     * parameter. */
-                    ConfNode *n0 = ConfNodeLookupChild(parent, value);
-                    if (n0 != NULL) {
-                        if (n0->allow_override) {
-                            ConfNodeRemove(n0);
-                        }
-                        else {
-                            state = CONF_VAL;
-                            goto next;
-                        }
-                    }
                     if (parent->is_seq) {
                         if (parent->val == NULL) {
                             parent->val = strdup(value);
                         }
                     }
-                    node = ConfNodeNew();
+                    ConfNode *n0 = ConfNodeLookupChild(parent, value);
-                    node->name = strdup(value);
+                    if (n0 != NULL) {
-                    TAILQ_INSERT_TAIL(&parent->head, node, next);
+                        node = n0;
+                    }
+                    else {
+                        node = ConfNodeNew();
+                        node->name = strdup(value);
+                        TAILQ_INSERT_TAIL(&parent->head, node, next);
+                    }
                     state = CONF_VAL;
                 }
                 else {
-                    node->val = strdup(value);
+                    if (node->allow_override) {
+                        if (node->val != NULL)
+                            free(node->val);
+                        node->val = strdup(value);
+                    }
                     state = CONF_KEY;
                 }
             }
@@ -147,7 +143,6 @@ ConfYamlParse(yaml_parser_t *parser, ConfNode *parent, int inseq)
             done = 1;
         }
 
-    next:
         yaml_event_delete(&event);
         continue;
 

src/conf.c

@@ -14,6 +14,9 @@
  * \todo Consider having the in-memory configuration database a direct
  *   reflection of the configuration file and moving command line
  *   parameters to a primary lookup table?
+ *
+ * \todo Get rid of allow override and go with a simpler first set,
+ *   stays approach?
  */
 
 #include <string.h>
@@ -62,6 +65,8 @@ ConfNodeNew(void)
             "Error allocating memory for new configuration node");
         exit(EXIT_FAILURE);
     }
+    /* By default we allow an override. */
+    new->allow_override = 1;
     TAILQ_INIT(&new->head);
 
     return new;
@@ -101,7 +106,7 @@ ConfNode *
 ConfGetNode(char *key)
 {
     ConfNode *node = root;
-    char *saveptr;
+    char *saveptr = NULL;
     char *token;
 
     /* Need to dup the key for tokenization... */
@@ -144,7 +149,7 @@ ConfSet(char *name, char *val, int allow_override)
     ConfNode *parent = root;
     ConfNode *node;
     char *token;
-    char *saveptr;
+    char *saveptr = NULL;
 
     /* First check if the node already exists. */
     node = ConfGetNode(name);
@@ -172,8 +177,13 @@ ConfSet(char *name, char *val, int allow_override)
                 TAILQ_INSERT_TAIL(&parent->head, node, next);
                 parent = node;
             }
+            else {
+                parent = node;
+            }
             token = strtok_r(NULL, ".", &saveptr);
             if (token == NULL) {
+                if (!node->allow_override)
+                    break;
                 if (node->val != NULL)
                     free(node->val);
                 node->val = strdup(val);
@@ -750,10 +760,46 @@ ConfNodeRemoveTest(void)
     return 1;
 }
 
+static int
+ConfSetTest(void)
+{
+    ConfCreateContextBackup();
+    ConfInit();
+
+    /* Set some value with 2 levels. */
+    if (ConfSet("one.two", "three", 1) != 1)
+        return 0;
+    ConfNode *n = ConfGetNode("one.two");
+    if (n == NULL)
+        return 0;
+
+    /* Set another 2 level parameter with the same first level, this
+     * used to trigger a bug that caused the second level of the name
+     * to become a first level node. */
+    if (ConfSet("one.three", "four", 1) != 1)
+        return 0;
+
+    n = ConfGetNode("one.three");
+    if (n == NULL)
+        return 0;
+
+    /* A top level node of "three" should not exist. */
+    n = ConfGetNode("three");
+    if (n != NULL)
+        return 0;
+
+    ConfDeInit();
+    ConfRestoreContextBackup();
+
+    return 1;
+}
+
+
 void
 ConfRegisterTests(void)
 {
     UtRegisterTest("ConfTestGetNonExistant", ConfTestGetNonExistant, 1);
+    UtRegisterTest("ConfSetTest", ConfSetTest, 1);
     UtRegisterTest("ConfTestSetAndGet", ConfTestSetAndGet, 1);
     UtRegisterTest("ConfTestOverrideValue1", ConfTestOverrideValue1, 1);
     UtRegisterTest("ConfTestOverrideValue2", ConfTestOverrideValue2, 1);

src/suricata.c

@@ -427,7 +427,6 @@ int main(int argc, char **argv)
                 }
             }
             else if(strcmp((long_opts[option_index]).name , "pfring-clusterid") == 0){
-                printf ("clusterid %s\n",optarg);
                 if (ConfSet("pfring.clusterid", optarg, 0) != 1) {
                     fprintf(stderr, "ERROR: Failed to set pfring clusterid.\n");
                     exit(EXIT_FAILURE);