Refactor settings code to avoid exit calls

Problem with exit calls is that settings/services POSTed during
run-time would stop the daemon completely.

diff --git a/src/http_command.c b/src/http_command.c
index e3c0c69..67e5a32 100644 (file)
--- a/src/http_command.c
+++ b/src/http_command.c
@@ -488,6 +488,7 @@ static void cmd_settings(struct http_channel *c)
     {
         xmlDoc *doc = xmlParseMemory(rq->content_buf, rq->content_len);
         xmlNode *root_n;
+        int ret;
         if (!doc)
         {
             error(rs, PAZPAR2_MALFORMED_SETTING, 0);
@@ -495,10 +496,14 @@ static void cmd_settings(struct http_channel *c)
             return;
         }
         root_n = xmlDocGetRootElement(doc);
-
-        settings_read_node_x(root_n, s->psession, apply_local_setting);
-
+        ret = settings_read_node_x(root_n, s->psession, apply_local_setting);
         xmlFreeDoc(doc);
+        if (ret)
+        {
+            error(rs, PAZPAR2_MALFORMED_SETTING, 0);
+            release_session(c,s);
+            return;
+        }            
     }
     if (process_settings(s->psession, rq, rs) < 0)
     {

diff --git a/src/pazpar2_config.c b/src/pazpar2_config.c
index bc5ba6d..75813d3 100644 (file)
--- a/src/pazpar2_config.c
+++ b/src/pazpar2_config.c
@@ -576,19 +576,22 @@ static struct conf_service *service_create_static(struct conf_server *server,
                     continue;
                 if (!strcmp((const char *) n->name, "settings"))
                 {
+                    int ret;
                     xmlChar *src = xmlGetProp(n, (xmlChar *) "src");
                     if (src)
                     {
                         WRBUF w = wrbuf_alloc();
                         conf_dir_path(server->config, w, (const char *) src);
-                        settings_read_file(service, wrbuf_cstr(w), pass);
+                        ret = settings_read_file(service, wrbuf_cstr(w), pass);
                         wrbuf_destroy(w);
                         xmlFree(src);
                     }
                     else
                     {
-                        settings_read_node(service, n, pass);
+                        ret = settings_read_node(service, n, pass);
                     }
+                    if (ret)
+                        return 0;
                 }
             }
         }
@@ -596,8 +599,9 @@ static struct conf_service *service_create_static(struct conf_server *server,
     return service;
 }
 
-static void inherit_server_settings(struct conf_service *s)
+static int inherit_server_settings(struct conf_service *s)
 {
+    int ret = 0;
     struct conf_server *server = s->server;
     if (!s->dictionary) /* service has no config settings ? */
     {
@@ -605,8 +609,10 @@ static void inherit_server_settings(struct conf_service *s)
         {
             /* inherit settings from server */
             init_settings(s);
-            settings_read_file(s, server->settings_fname, 1);
-            settings_read_file(s, server->settings_fname, 2);
+            if (settings_read_file(s, server->settings_fname, 1))
+                ret = -1;
+            if (settings_read_file(s, server->settings_fname, 2))
+                ret = -1;
         }
         else
         {
@@ -630,6 +636,7 @@ static void inherit_server_settings(struct conf_service *s)
             s->charsets = pp2_charset_fact_create();
         }
     }
+    return ret;
 }
 
 struct conf_service *service_create(struct conf_server *server,

diff --git a/src/settings.c b/src/settings.c
index 1f9246c..a1b15eb 100644 (file)
--- a/src/settings.c
+++ b/src/settings.c
@@ -162,69 +162,53 @@ static int isdir(const char *path)
 }
 
 // Read settings from an XML file, calling handler function for each setting
-void settings_read_node_x(xmlNode *n,
-                          void *client_data,
-                          void (*fun)(void *client_data,
-                                      struct setting *set))
+int settings_read_node_x(xmlNode *n,
+                         void *client_data,
+                         void (*fun)(void *client_data,
+                                     struct setting *set))
 {
-    xmlChar *namea, *targeta, *valuea, *usera, *precedencea;
+    int ret_val = 0; /* success */
+    char *namea = (char *) xmlGetProp(n, (xmlChar *) "name");
+    char *targeta = (char *) xmlGetProp(n, (xmlChar *) "target");
+    char *valuea = (char *) xmlGetProp(n, (xmlChar *) "value");
+    char *usera = (char *) xmlGetProp(n, (xmlChar *) "user");
+    char *precedencea = (char *) xmlGetProp(n, (xmlChar *) "precedence");
 
-    namea = xmlGetProp(n, (xmlChar *) "name");
-    targeta = xmlGetProp(n, (xmlChar *) "target");
-    valuea = xmlGetProp(n, (xmlChar *) "value");
-    usera = xmlGetProp(n, (xmlChar *) "user");
-    precedencea = xmlGetProp(n, (xmlChar *) "precedence");
     for (n = n->children; n; n = n->next)
     {
         if (n->type != XML_ELEMENT_NODE)
             continue;
         if (!strcmp((const char *) n->name, "set"))
         {
-            char *name, *target, *value, *user, *precedence;
+            struct setting set;
+            char *name = (char *) xmlGetProp(n, (xmlChar *) "name");
+            char *target = (char *) xmlGetProp(n, (xmlChar *) "target");
+            char *value = (char *) xmlGetProp(n, (xmlChar *) "value");
+            char *user = (char *) xmlGetProp(n, (xmlChar *) "user");
+            char *precedence = (char *) xmlGetProp(n, (xmlChar *) "precedence");
+
+            if (precedence)
+                set.precedence = atoi((char *) precedence);
+            else if (precedencea)
+                set.precedence = atoi((char *) precedencea);
+            else
+                set.precedence = 0;
 
-            name = (char *) xmlGetProp(n, (xmlChar *) "name");
-            target = (char *) xmlGetProp(n, (xmlChar *) "target");
-            value = (char *) xmlGetProp(n, (xmlChar *) "value");
-            user = (char *) xmlGetProp(n, (xmlChar *) "user");
-            precedence = (char *) xmlGetProp(n, (xmlChar *) "precedence");
+            set.target = target ? target : targeta;
+            set.name = name ? name : namea;
+            set.value = value ? value : valuea;
+            set.next = 0;
 
-            if ((!name && !namea) || (!value && !valuea) || (!target && !targeta))
-            {
-                yaz_log(YLOG_FATAL, "set must specify name, value, and target");
-                exit(1);
-            }
+            if (set.name && set.value && set.target)
+                (*fun)(client_data, &set);
             else
             {
-                struct setting set;
-                char nameb[1024];
-                char targetb[1024];
-                char valueb[1024];
-
-                // Copy everything into a temporary buffer -- we decide
-                // later if we are keeping it.
-                if (precedence)
-                    set.precedence = atoi((char *) precedence);
-                else if (precedencea)
-                    set.precedence = atoi((char *) precedencea);
+                if (set.name)
+                    yaz_log(YLOG_WARN, "missing value and/or target for "
+                            "setting name=%s", set.name);
                 else
-                    set.precedence = 0;
-                if (target)
-                    strcpy(targetb, target);
-                else
-                    strcpy(targetb, (const char *) targeta);
-                set.target = targetb;
-                if (name)
-                    strcpy(nameb, name);
-                else
-                    strcpy(nameb, (const char *) namea);
-                set.name = nameb;
-                if (value)
-                    strcpy(valueb, value);
-                else
-                    strcpy(valueb, (const char *) valuea);
-                set.value = valueb;
-                set.next = 0;
-                (*fun)(client_data, &set);
+                    yaz_log(YLOG_WARN, "missing name/value/target for setting");
+                ret_val = -1;
             }
             xmlFree(name);
             xmlFree(precedence);
@@ -234,8 +218,9 @@ void settings_read_node_x(xmlNode *n,
         }
         else
         {
-            yaz_log(YLOG_FATAL, "Unknown element %s in settings file", (char*) n->name);
-            exit(1);
+            yaz_log(YLOG_WARN, "Unknown element %s in settings file", 
+                    (char*) n->name);
+            ret_val = -1;
         }
     }
     xmlFree(namea);
@@ -243,35 +228,39 @@ void settings_read_node_x(xmlNode *n,
     xmlFree(valuea);
     xmlFree(usera);
     xmlFree(targeta);
+    return ret_val;
 }
  
-static void read_settings_file(const char *path,
-                               void *client_data,
-                               void (*fun)(void *client_data,
-                                           struct setting *set))
+static int read_settings_file(const char *path,
+                              void *client_data,
+                              void (*fun)(void *client_data,
+                                          struct setting *set))
 {
     xmlDoc *doc = xmlParseFile(path);
     xmlNode *n;
+    int ret;
 
     if (!doc)
     {
         yaz_log(YLOG_FATAL, "Failed to parse %s", path);
-        exit(1);
+        return -1;
     }
     n = xmlDocGetRootElement(doc);
-    settings_read_node_x(n, client_data, fun);
+    ret = settings_read_node_x(n, client_data, fun);
 
     xmlFreeDoc(doc);
+    return ret;
 }
 
 
 // Recursively read files or directories, invoking a 
 // callback for each one
-static void read_settings(const char *path,
+static int read_settings(const char *path,
                           void *client_data,
                           void (*fun)(void *client_data,
                                       struct setting *set))
 {
+    int ret = 0;
     DIR *d;
     struct dirent *de;
     char *dot;
@@ -281,7 +270,7 @@ static void read_settings(const char *path,
         if (!(d = opendir(path)))
         {
             yaz_log(YLOG_FATAL|YLOG_ERRNO, "%s", path);
-            exit(1);
+            return -1;
         }
         while ((de = readdir(d)))
         {
@@ -289,12 +278,14 @@ static void read_settings(const char *path,
             if (*de->d_name == '.' || !strcmp(de->d_name, "CVS"))
                 continue;
             sprintf(tmp, "%s/%s", path, de->d_name);
-            read_settings(tmp, client_data, fun);
+            if (read_settings(tmp, client_data, fun))
+                ret = -1;
         }
         closedir(d);
     }
     else if ((dot = strrchr(path, '.')) && !strcmp(dot + 1, "xml"))
-        read_settings_file(path, client_data, fun);
+        ret = read_settings_file(path, client_data, fun);
+    return ret;
 }
 
 // Determines if a ZURL is a wildcard, and what kind
@@ -456,22 +447,22 @@ void init_settings(struct conf_service *service)
     initialize_soft_settings(service);
 }
 
-void settings_read_file(struct conf_service *service, const char *path,
-                        int pass)
+int settings_read_file(struct conf_service *service, const char *path,
+                       int pass)
 {
     if (pass == 1)
-        read_settings(path, service, prepare_target_dictionary);
+        return read_settings(path, service, prepare_target_dictionary);
     else
-        read_settings(path, service, update_databases);
+        return read_settings(path, service, update_databases);
 }
 
-void settings_read_node(struct conf_service *service, xmlNode *n,
+int settings_read_node(struct conf_service *service, xmlNode *n,
                         int pass)
 {
     if (pass == 1)
-        settings_read_node_x(n, service, prepare_target_dictionary);
+        return settings_read_node_x(n, service, prepare_target_dictionary);
     else
-        settings_read_node_x(n, service, update_databases);
+        return settings_read_node_x(n, service, update_databases);
 }
 
 /*

diff --git a/src/settings.h b/src/settings.h
index cbba02c..39823f0 100644 (file)
--- a/src/settings.h
+++ b/src/settings.h
@@ -62,18 +62,18 @@ struct setting
     struct setting *next;
 };
 
-void settings_read_file(struct conf_service *service, const char *path,
-                        int pass);
-void settings_read_node(struct conf_service *service, xmlNode *n,
-                        int pass);
+int settings_read_file(struct conf_service *service, const char *path,
+                       int pass);
+int settings_read_node(struct conf_service *service, xmlNode *n,
+                       int pass);
 int settings_num(struct conf_service *service);
 int settings_create_offset(struct conf_service *service, const char *name);
 int settings_lookup_offset(struct conf_service *service, const char *name);
 void init_settings(struct conf_service *service);
-void settings_read_node_x(xmlNode *n,
-                          void *client_data,
-                          void (*fun)(void *client_data,
-                                      struct setting *set));
+int settings_read_node_x(xmlNode *n,
+                         void *client_data,
+                         void (*fun)(void *client_data,
+                                     struct setting *set));
 void expand_settings_array(struct setting ***set_ar, int *num, int offset,
                            NMEM nmem);