From: Jakub Jankowski <shasta@toxcorp.com>
Date: Mon, 5 Nov 2018 23:30:26 +0000 (+0100)
Subject: curl plugin: address code review comments
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=c6867a1cd3d6f340b45019fd9166117183da6f22;p=collectd.git

curl plugin: address code review comments
---

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index f62417b8..56be262c 100644
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -1841,10 +1841,12 @@ extract information from this data, non-binary data is a big plus here ;)
 IP version to resolve URL to. Useful in cases when hostname in URL resolves
 to both IPv4 and IPv6 addresses, and you are interested in using one of them
 specifically.
-Use C<ipv4> to enforce IPv4, C<ipv6> to enforce IPv6. If C<libcurl> is compiled
-without IPv6 support, the latter will result in a warning, and fallback to C<any>.
-If C<Type> cannot be parsed, C<any> will be used as well (all IP versions that
-your system allows).
+Use C<ipv4> to enforce IPv4, C<ipv6> to enforce IPv6, or C<any> to keep the
+default behavior of resolving addresses to all IP versions your system allows.
+If C<libcurl> is compiled without IPv6 support, using C<ipv6> will result in
+a warning and fallback to C<any>.
+If C<Type> cannot be parsed, a warning will be printed and the whole B<Page>
+block will be ignored.
 
 =item B<User> I<Name>
 
diff --git a/src/curl.c b/src/curl.c
index 5c30ab7d..950e30b7 100644
--- a/src/curl.c
+++ b/src/curl.c
@@ -400,8 +400,6 @@ static int cc_config_add_page(oconfig_item_t *ci) /* {{{ */
   cdtime_t interval = 0;
   web_page_t *page;
   int status;
-  char *af = NULL;
-  curl_version_info_data *curl_info = curl_version_info(CURLVERSION_NOW);
 
   if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING)) {
     WARNING("curl plugin: `Page' blocks need exactly one string argument.");
@@ -443,16 +441,29 @@ static int cc_config_add_page(oconfig_item_t *ci) /* {{{ */
     else if (strcasecmp("URL", child->key) == 0)
       status = cf_util_get_string(child, &page->url);
     else if (strcasecmp("AddressFamily", child->key) == 0) {
+      char *af = NULL;
       status = cf_util_get_string(child, &af);
       if (status != 0 || af == NULL) {
+        WARNING("curl plugin: Cannot parse value of `%s' "
+                "for instance `%s'.", child->key, page->instance);
+      } else if (strcasecmp("any", af) == 0) {
         page->address_family = CURL_IPRESOLVE_WHATEVER;
-      } else if (strcasecmp("inet4", af) == 0 || strcasecmp("ipv4", af) == 0) {
+      } else if (strcasecmp("ipv4", af) == 0) {
         page->address_family = CURL_IPRESOLVE_V4;
-      } else if (strcasecmp("inet6", af) == 0 || strcasecmp("ipv6", af) == 0) {
+      } else if (strcasecmp("ipv6", af) == 0) {
+        /* If curl supports ipv6, use it. If not, log a warning and
+         * fall back to default - don't set status to non-zero.
+         */
+        curl_version_info_data *curl_info = curl_version_info(CURLVERSION_NOW);
         if (curl_info->features & CURL_VERSION_IPV6)
           page->address_family = CURL_IPRESOLVE_V6;
         else
-          WARNING("curl plugin: IPv6 not supported by this libCURL.");
+          WARNING("curl plugin: IPv6 not supported by this libCURL. "
+                  "Using fallback `any'.");
+      } else {
+        WARNING("curl plugin: Unsupported value of `%s' "
+                "for instance `%s'.", child->key, page->instance);
+        status = -1;
       }
     } else if (strcasecmp("User", child->key) == 0)
       status = cf_util_get_string(child, &page->user);