X-Git-Url: https://git.octo.it/?p=collectd.git;a=blobdiff_plain;f=src%2Fcurl_xml.c;h=19ae5f462de8a4cd64c263ddf7c62a56436909a7;hp=e1f5ed3ce0a734185c39bd0151d074b15d8e6121;hb=77ca1a45bab2f6adf9301723d0db68e5813a6d98;hpb=cc7bfb31a63fa67e6e82421eb0a139a39924e64a

diff --git a/src/curl_xml.c b/src/curl_xml.c
index e1f5ed3c..19ae5f46 100644
--- a/src/curl_xml.c
+++ b/src/curl_xml.c
@@ -157,13 +157,14 @@ static void cx_xpath_list_free(llist_t *list) /* {{{ */
   while (le != NULL) {
     llentry_t *le_next = le->next;
 
+    /* this also frees xpath->path used for le->key */
     cx_xpath_free(le->value);
 
     le = le_next;
   }
 
   llist_destroy(list);
-} /* }}} void cx_list_free */
+} /* }}} void cx_xpath_list_free */
 
 static void cx_free(void *arg) /* {{{ */
 {
@@ -239,8 +240,8 @@ static int cx_check_type(const data_set_t *ds, cx_xpath_t *xpath) /* {{{ */
   }
 
   if (ds->ds_num != xpath->values_len) {
-    WARNING("curl_xml plugin: DataSet `%s' requires %zu values, but config "
-            "talks about %zu",
+    WARNING("curl_xml plugin: DataSet `%s' requires %" PRIsz
+            " values, but config talks about %" PRIsz,
             xpath->type, ds->ds_num, xpath->values_len);
     return -1;
   }
@@ -275,50 +276,65 @@ static int cx_if_not_text_node(xmlNodePtr node) /* {{{ */
   return -1;
 } /* }}} cx_if_not_text_node */
 
-static int cx_handle_single_value_xpath(xmlXPathContextPtr xpath_ctx, /* {{{ */
-                                        cx_xpath_t *xpath, const data_set_t *ds,
-                                        value_list_t *vl, int index) {
-  xmlXPathObjectPtr values_node_obj;
-  xmlNodeSetPtr values_node;
-  int tmp_size;
-  char *node_value;
-
-  values_node_obj = cx_evaluate_xpath(xpath_ctx, xpath->values[index].path);
+/*
+ * Returned value should be freed with xmlFree().
+ */
+static char *cx_get_text_node_value(xmlXPathContextPtr xpath_ctx, /* {{{ */
+                                    char *expr, const char *from_option) {
+  xmlXPathObjectPtr values_node_obj = cx_evaluate_xpath(xpath_ctx, expr);
   if (values_node_obj == NULL)
-    return -1; /* Error already logged. */
+    return NULL; /* Error already logged. */
 
-  values_node = values_node_obj->nodesetval;
-  tmp_size = (values_node) ? values_node->nodeNr : 0;
+  xmlNodeSetPtr values_node = values_node_obj->nodesetval;
+  size_t tmp_size = (values_node) ? values_node->nodeNr : 0;
 
   if (tmp_size == 0) {
     WARNING("curl_xml plugin: "
-            "relative xpath expression \"%s\" doesn't match any of the nodes. "
-            "Skipping...",
-            xpath->values[index].path);
+            "relative xpath expression \"%s\" from '%s' doesn't match "
+            "any of the nodes.",
+            expr, from_option);
     xmlXPathFreeObject(values_node_obj);
-    return -1;
+    return NULL;
   }
 
   if (tmp_size > 1) {
     WARNING("curl_xml plugin: "
-            "relative xpath expression \"%s\" is expected to return "
-            "only one node. Skipping...",
-            xpath->values[index].path);
+            "relative xpath expression \"%s\" from '%s' is expected to return "
+            "only one text node. Skipping the node.",
+            expr, from_option);
     xmlXPathFreeObject(values_node_obj);
-    return -1;
+    return NULL;
   }
 
   /* ignoring the element if other than textnode/attribute*/
   if (cx_if_not_text_node(values_node->nodeTab[0])) {
     WARNING("curl_xml plugin: "
-            "relative xpath expression \"%s\" is expected to return "
-            "only text/attribute node which is not the case. Skipping...",
-            xpath->values[index].path);
+            "relative xpath expression \"%s\" from '%s' is expected to return "
+            "only text/attribute node which is not the case. "
+            "Skipping the node.",
+            expr, from_option);
     xmlXPathFreeObject(values_node_obj);
-    return -1;
+    return NULL;
   }
 
-  node_value = (char *)xmlNodeGetContent(values_node->nodeTab[0]);
+  char *node_value = (char *)xmlNodeGetContent(values_node->nodeTab[0]);
+
+  /* free up object */
+  xmlXPathFreeObject(values_node_obj);
+
+  return node_value;
+} /* }}} char * cx_get_text_node_value */
+
+static int cx_handle_single_value_xpath(xmlXPathContextPtr xpath_ctx, /* {{{ */
+                                        cx_xpath_t *xpath, const data_set_t *ds,
+                                        value_list_t *vl, int index) {
+
+  char *node_value = cx_get_text_node_value(
+      xpath_ctx, xpath->values[index].path, "ValuesFrom");
+
+  if (node_value == NULL)
+    return -1;
+
   switch (ds->ds[index].type) {
   case DS_TYPE_COUNTER:
     vl->values[index].counter =
@@ -340,9 +356,7 @@ static int cx_handle_single_value_xpath(xmlXPathContextPtr xpath_ctx, /* {{{ */
                                               /* endptr = */ NULL);
   }
 
-  /* free up object */
-  xmlXPathFreeObject(values_node_obj);
-  sfree(node_value);
+  xmlFree(node_value);
 
   /* We have reached here which means that
    * we have got something to work */
@@ -373,128 +387,35 @@ static int cx_handle_all_value_xpaths(xmlXPathContextPtr xpath_ctx, /* {{{ */
 static int cx_handle_instance_xpath(xmlXPathContextPtr xpath_ctx, /* {{{ */
                                     cx_xpath_t *xpath, value_list_t *vl) {
 
-  xmlXPathObjectPtr instance_node_obj = NULL;
-  xmlNodeSetPtr instance_node = NULL;
-
-  memset(vl->type_instance, 0, sizeof(vl->type_instance));
-
-  /* instance has to be an xpath expression */
+  /* Handle type instance */
   if (xpath->instance != NULL) {
-    int tmp_size;
-
-    instance_node_obj = cx_evaluate_xpath(xpath_ctx, xpath->instance);
-    if (instance_node_obj == NULL)
-      return -1; /* error is logged already */
-
-    instance_node = instance_node_obj->nodesetval;
-    tmp_size = (instance_node) ? instance_node->nodeNr : 0;
-
-    if (tmp_size <= 0) {
-      WARNING(
-          "curl_xml plugin: "
-          "relative xpath expression for 'InstanceFrom' \"%s\" doesn't match "
-          "any of the nodes. Skipping the node.",
-          xpath->instance);
-      xmlXPathFreeObject(instance_node_obj);
+    char *node_value =
+        cx_get_text_node_value(xpath_ctx, xpath->instance, "InstanceFrom");
+    if (node_value == NULL)
       return -1;
-    }
-
-    if (tmp_size > 1) {
-      WARNING("curl_xml plugin: "
-              "relative xpath expression for 'InstanceFrom' \"%s\" is expected "
-              "to return only one text node. Skipping the node.",
-              xpath->instance);
-      xmlXPathFreeObject(instance_node_obj);
-      return -1;
-    }
 
-    /* ignoring the element if other than textnode/attribute */
-    if (cx_if_not_text_node(instance_node->nodeTab[0])) {
-      WARNING("curl_xml plugin: "
-              "relative xpath expression \"%s\" is expected to return only "
-              "text node "
-              "which is not the case. Skipping the node.",
-              xpath->instance);
-      xmlXPathFreeObject(instance_node_obj);
-      return -1;
-    }
-  } /* if (xpath->instance != NULL) */
-
-  if (xpath->instance_prefix != NULL) {
-    if (instance_node != NULL) {
-      char *node_value = (char *)xmlNodeGetContent(instance_node->nodeTab[0]);
+    if (xpath->instance_prefix != NULL)
       snprintf(vl->type_instance, sizeof(vl->type_instance), "%s%s",
                xpath->instance_prefix, node_value);
-      sfree(node_value);
-    } else
-      sstrncpy(vl->type_instance, xpath->instance_prefix,
-               sizeof(vl->type_instance));
-  } else {
-    /* If instance_prefix and instance_node are NULL, then
-     * don't set the type_instance */
-    if (instance_node != NULL) {
-      char *node_value = (char *)xmlNodeGetContent(instance_node->nodeTab[0]);
+    else
       sstrncpy(vl->type_instance, node_value, sizeof(vl->type_instance));
-      sfree(node_value);
-    }
-  }
 
-  /* Free `instance_node_obj' this late, because `instance_node' points to
-   * somewhere inside this structure. */
-  xmlXPathFreeObject(instance_node_obj);
+    xmlFree(node_value);
+  } else if (xpath->instance_prefix != NULL)
+    sstrncpy(vl->type_instance, xpath->instance_prefix,
+             sizeof(vl->type_instance));
 
-  /* Part 2, handle PluginInstanceFrom */
-  instance_node_obj = NULL;
-  instance_node = NULL;
-
-  /* plugin_instance_from has to be an xpath expression */
+  /* Handle plugin instance */
   if (xpath->plugin_instance_from != NULL) {
-    instance_node_obj =
-        cx_evaluate_xpath(xpath_ctx, xpath->plugin_instance_from);
-    if (instance_node_obj == NULL)
-      return -1; /* error is already logged */
-
-    instance_node = instance_node_obj->nodesetval;
-    int tmp_size = (instance_node) ? instance_node->nodeNr : 0;
-
-    if (tmp_size <= 0) {
-      WARNING("curl_xml plugin: "
-              "relative xpath expression for 'PluginInstanceFrom' \"%s\" "
-              "doesn't match any of the nodes. Skipping the node.",
-              xpath->plugin_instance_from);
-      xmlXPathFreeObject(instance_node_obj);
-      return -1;
-    }
+    char *node_value = cx_get_text_node_value(
+        xpath_ctx, xpath->plugin_instance_from, "PluginInstanceFrom");
 
-    if (tmp_size > 1) {
-      WARNING("curl_xml plugin: "
-              "relative xpath expression for 'PluginInstanceFrom' \"%s\" "
-              "is expected to return only one text node. Skipping the node.",
-              xpath->plugin_instance_from);
-      xmlXPathFreeObject(instance_node_obj);
+    if (node_value == NULL)
       return -1;
-    }
 
-    /* ignoring the element if other than textnode/attribute */
-    if (cx_if_not_text_node(instance_node->nodeTab[0])) {
-      WARNING("curl_xml plugin: "
-              "relative xpath expression \"%s\" is expected to return only "
-              "text node which is not the case. Skipping the node.",
-              xpath->plugin_instance_from);
-      xmlXPathFreeObject(instance_node_obj);
-      return -1;
-    }
-
-    if (instance_node != NULL) {
-      char *node_value = (char *)xmlNodeGetContent(instance_node->nodeTab[0]);
-      sstrncpy(vl->plugin_instance, node_value, sizeof(vl->plugin_instance));
-      sfree(node_value);
-    }
-
-    /* Free `instance_node_obj' this late, because `instance_node' points to
-     * somewhere inside this structure. */
-    xmlXPathFreeObject(instance_node_obj);
-  } /* if (xpath->plugin_instance_from != NULL) */
+    sstrncpy(vl->plugin_instance, node_value, sizeof(vl->plugin_instance));
+    xmlFree(node_value);
+  }
 
   return 0;
 } /* }}} int cx_handle_instance_xpath */
@@ -531,6 +452,7 @@ static int cx_handle_xpath(const cx_t *db, /* {{{ */
           "since the base xpath expression \"%s\" "
           "returned multiple results. Skipping the xpath block...",
           xpath->path);
+    xmlXPathFreeObject(base_node_obj);
     return -1;
   }
 
@@ -747,13 +669,10 @@ static int cx_config_add_xpath(cx_t *db, oconfig_item_t *ci) /* {{{ */
     return -1;
   }
 
-  if (db->xpath_list == NULL) {
-    db->xpath_list = llist_create();
-    if (db->xpath_list == NULL) {
-      ERROR("curl_xml plugin: list creation failed.");
-      cx_xpath_free(xpath);
-      return -1;
-    }
+  if (xpath->values_len == 0) {
+    WARNING("curl_xml plugin: `ValuesFrom' missing in `xpath' block.");
+    cx_xpath_free(xpath);
+    return -1;
   }
 
   llentry_t *le = llentry_create(xpath->path, xpath);
@@ -867,8 +786,6 @@ static int cx_init_curl(cx_t *db) /* {{{ */
 
 static int cx_config_add_url(oconfig_item_t *ci) /* {{{ */
 {
-  int status = 0;
-
   if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING)) {
     WARNING("curl_xml plugin: The `URL' block "
             "needs exactly one string argument.");
@@ -881,22 +798,31 @@ static int cx_config_add_url(oconfig_item_t *ci) /* {{{ */
     return -1;
   }
 
-  db->timeout = -1;
+  db->instance = strdup("default");
+  if (db->instance == NULL) {
+    ERROR("curl_xml plugin: strdup failed.");
+    sfree(db);
+    return -1;
+  }
 
-  if (strcasecmp("URL", ci->key) == 0) {
-    status = cf_util_get_string(ci, &db->url);
-    if (status != 0) {
-      sfree(db);
-      return status;
-    }
-  } else {
-    ERROR("curl_xml plugin: cx_config: "
-          "Invalid key: %s",
-          ci->key);
-    cx_free(db);
+  db->xpath_list = llist_create();
+  if (db->xpath_list == NULL) {
+    ERROR("curl_xml plugin: list creation failed.");
+    sfree(db->instance);
+    sfree(db);
     return -1;
   }
 
+  db->timeout = -1;
+
+  int status = cf_util_get_string(ci, &db->url);
+  if (status != 0) {
+    llist_destroy(db->xpath_list);
+    sfree(db->instance);
+    sfree(db);
+    return status;
+  }
+
   /* Fill the `cx_t' structure.. */
   for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *child = ci->children + i;
@@ -942,37 +868,34 @@ static int cx_config_add_url(oconfig_item_t *ci) /* {{{ */
       break;
   }
 
-  if (status == 0) {
-    if (db->xpath_list == NULL) {
-      WARNING("curl_xml plugin: No (valid) `xpath' block "
-              "within `URL' block `%s'.",
-              db->url);
-      status = -1;
-    }
-    if (status == 0)
-      status = cx_init_curl(db);
+  if (status != 0) {
+    cx_free(db);
+    return status;
   }
 
-  /* If all went well, register this database for reading */
-  if (status == 0) {
-    if (db->instance == NULL)
-      db->instance = strdup("default");
-
-    DEBUG("curl_xml plugin: Registering new read callback: %s", db->instance);
-
-    char *cb_name = ssnprintf_alloc("curl_xml-%s-%s", db->instance, db->url);
+  if (llist_size(db->xpath_list) == 0) {
+    WARNING("curl_xml plugin: No `xpath' block within `URL' block `%s'.",
+            db->url);
+    cx_free(db);
+    return -1;
+  }
 
-    plugin_register_complex_read(/* group = */ "curl_xml", cb_name, cx_read,
-                                 /* interval = */ 0,
-                                 &(user_data_t){
-                                     .data = db, .free_func = cx_free,
-                                 });
-    sfree(cb_name);
-  } else {
+  if (cx_init_curl(db) != 0) {
     cx_free(db);
     return -1;
   }
 
+  /* If all went well, register this database for reading */
+  DEBUG("curl_xml plugin: Registering new read callback: %s", db->instance);
+
+  char *cb_name = ssnprintf_alloc("curl_xml-%s-%s", db->instance, db->url);
+
+  plugin_register_complex_read(/* group = */ "curl_xml", cb_name, cx_read,
+                               /* interval = */ 0,
+                               &(user_data_t){
+                                   .data = db, .free_func = cx_free,
+                               });
+  sfree(cb_name);
   return 0;
 } /* }}} int cx_config_add_url */