curl plugin: address code review comments

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index f62417b..56be262 100644 (file)
--- 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.
 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>
 
 
 =item B<User> I<Name>
 

diff --git a/src/curl.c b/src/curl.c
index 5c30ab7..950e30b 100644 (file)
--- 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;
   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.");
 
   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) {
     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) {
       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;
         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;
         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
         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);
       }
     } else if (strcasecmp("User", child->key) == 0)
       status = cf_util_get_string(child, &page->user);