Merge remote-tracking branch 'github/pr/2467'
authorFlorian Forster <octo@collectd.org>
Thu, 19 Oct 2017 07:21:50 +0000 (09:21 +0200)
committerFlorian Forster <octo@collectd.org>
Thu, 19 Oct 2017 07:21:50 +0000 (09:21 +0200)
Makefile.am
src/daemon/collectd.c
src/daemon/collectd.h
src/daemon/configfile.c
src/daemon/globals.c [new file with mode: 0644]
src/daemon/globals.h [new file with mode: 0644]
src/daemon/plugin.h
src/daemon/plugin_mock.c
src/perl.c

index 04636b3..ae027a3 100644 (file)
@@ -199,6 +199,8 @@ collectd_SOURCES = \
        src/daemon/configfile.h \
        src/daemon/filter_chain.c \
        src/daemon/filter_chain.h \
+       src/daemon/globals.c \
+       src/daemon/globals.h \
        src/daemon/meta_data.c \
        src/daemon/meta_data.h \
        src/daemon/plugin.c \
index af8fb56..dd9b12f 100644 (file)
 #define COLLECTD_LOCALE "C"
 #endif
 
-/*
- * Global variables
- */
-char hostname_g[DATA_MAX_NAME_LEN];
-cdtime_t interval_g;
-int timeout_g;
-#if HAVE_LIBKSTAT
-kstat_ctl_t *kc;
-#endif /* HAVE_LIBKSTAT */
-
 static int loop = 0;
 
 static void *do_flush(void __attribute__((unused)) * arg) {
@@ -91,13 +81,19 @@ static int init_hostname(void) {
   struct addrinfo *ai_list;
   int status;
 
+  long hostname_len = sysconf(_SC_HOST_NAME_MAX);
+  if (hostname_len == -1) {
+    hostname_len = NI_MAXHOST;
+  }
+  char hostname[hostname_len];
+
   str = global_option_get("Hostname");
   if ((str != NULL) && (str[0] != 0)) {
-    sstrncpy(hostname_g, str, sizeof(hostname_g));
+    hostname_set(str);
     return 0;
   }
 
-  if (gethostname(hostname_g, sizeof(hostname_g)) != 0) {
+  if (gethostname(hostname, hostname_len) != 0) {
     fprintf(stderr, "`gethostname' failed and no "
                     "hostname was configured.\n");
     return -1;
@@ -109,14 +105,14 @@ static int init_hostname(void) {
 
   struct addrinfo ai_hints = {.ai_flags = AI_CANONNAME};
 
-  status = getaddrinfo(hostname_g, NULL, &ai_hints, &ai_list);
+  status = getaddrinfo(hostname, NULL, &ai_hints, &ai_list);
   if (status != 0) {
     ERROR("Looking up \"%s\" failed. You have set the "
           "\"FQDNLookup\" option, but I cannot resolve "
           "my hostname to a fully qualified domain "
           "name. Please fix the network "
           "configuration.",
-          hostname_g);
+          hostname);
     return -1;
   }
 
@@ -125,7 +121,7 @@ static int init_hostname(void) {
     if (ai_ptr->ai_canonname == NULL)
       continue;
 
-    sstrncpy(hostname_g, ai_ptr->ai_canonname, sizeof(hostname_g));
+    hostname_set(ai_ptr->ai_canonname);
     break;
   }
 
@@ -455,23 +451,19 @@ static int notify_systemd(void) {
 }
 #endif /* KERNEL_LINUX */
 
-int main(int argc, char **argv) {
-  const char *configfile = CONFIGFILE;
-  int test_config = 0;
-  int test_readall = 0;
-  const char *basedir;
-  _Bool opt_create_basedir = 1;
-#if COLLECT_DAEMON
-  pid_t pid;
-  int daemonize = 1;
-#endif
-  int exit_status = 0;
+struct cmdline_config {
+  _Bool test_config;
+  _Bool test_readall;
+  _Bool create_basedir;
+  const char *configfile;
+  _Bool daemonize;
+};
 
+void read_cmdline(int argc, char **argv, struct cmdline_config *config) {
   /* read options */
   while (1) {
     int c;
-
-    c = getopt(argc, argv, "BhtTC:"
+    c = getopt(argc, argv, "htTC:"
 #if COLLECT_DAEMON
                            "fP:"
 #endif
@@ -482,19 +474,19 @@ int main(int argc, char **argv) {
 
     switch (c) {
     case 'B':
-      opt_create_basedir = 0;
+      config->create_basedir = 0;
       break;
     case 'C':
-      configfile = optarg;
+      config->configfile = optarg;
       break;
     case 't':
-      test_config = 1;
+      config->test_config = 1;
       break;
     case 'T':
-      test_readall = 1;
+      config->test_readall = 1;
       global_option_set("ReadThreads", "-1", 1);
 #if COLLECT_DAEMON
-      daemonize = 0;
+      config->daemonize = 0;
 #endif /* COLLECT_DAEMON */
       break;
 #if COLLECT_DAEMON
@@ -502,7 +494,7 @@ int main(int argc, char **argv) {
       global_option_set("PIDFile", optarg, 1);
       break;
     case 'f':
-      daemonize = 0;
+      config->daemonize = 0;
       break;
 #endif /* COLLECT_DAEMON */
     case 'h':
@@ -512,19 +504,17 @@ int main(int argc, char **argv) {
       exit_usage(1);
     } /* switch (c) */
   }   /* while (1) */
+}
 
-  if (optind < argc)
-    exit_usage(1);
-
-  plugin_init_ctx();
-
+int configure_collectd(struct cmdline_config *config) {
+  const char *basedir;
   /*
    * Read options from the config file, the environment and the command
    * line (in that order, with later options overwriting previous ones in
    * general).
    * Also, this will automatically load modules.
    */
-  if (cf_read(configfile)) {
+  if (cf_read(config->configfile)) {
     fprintf(stderr, "Error: Reading the config file failed!\n"
                     "Read the logs for details.\n");
     return 1;
@@ -538,23 +528,47 @@ int main(int argc, char **argv) {
     fprintf(stderr,
             "Don't have a basedir to use. This should not happen. Ever.");
     return 1;
-  } else if (change_basedir(basedir, opt_create_basedir)) {
+  } else if (change_basedir(basedir, config->create_basedir)) {
     fprintf(stderr, "Error: Unable to change to directory `%s'.\n", basedir);
     return 1;
   }
 
   /*
-   * Set global variables or, if that failes, exit. We cannot run with
+   * Set global variables or, if that fails, exit. We cannot run with
    * them being uninitialized. If nothing is configured, then defaults
    * are being used. So this means that the user has actually done
    * something wrong.
    */
   if (init_global_variables() != 0)
-    exit(EXIT_FAILURE);
+    return 1;
+
+  return 0;
+}
+
+int main(int argc, char **argv) {
+#if COLLECT_DAEMON
+  pid_t pid;
+#endif
+  int exit_status = 0;
 
-  if (test_config)
+  struct cmdline_config config = {
+      .daemonize = 1, .create_basedir = 1, .configfile = CONFIGFILE,
+  };
+
+  read_cmdline(argc, argv, &config);
+
+  if (config.test_config)
     return 0;
 
+  if (optind < argc)
+    exit_usage(1);
+
+  plugin_init_ctx();
+
+  int status;
+  if ((status = configure_collectd(&config)) != 0)
+    exit(EXIT_FAILURE);
+
 #if COLLECT_DAEMON
   /*
    * fork off child
@@ -567,7 +581,7 @@ int main(int argc, char **argv) {
    * Only daemonize if we're not being supervised
    * by upstart or systemd (when using Linux).
    */
-  if (daemonize
+  if (config.daemonize
 #ifdef KERNEL_LINUX
       && notify_upstart() == 0 && notify_systemd() == 0
 #endif
@@ -617,7 +631,7 @@ int main(int argc, char **argv) {
             status);
       return 1;
     }
-  }    /* if (daemonize) */
+  }    /* if (config.daemonize) */
 #endif /* COLLECT_DAEMON */
 
   struct sigaction sig_pipe_action = {.sa_handler = SIG_IGN};
@@ -662,7 +676,7 @@ int main(int argc, char **argv) {
     exit_status = 1;
   }
 
-  if (test_readall) {
+  if (config.test_readall) {
     if (plugin_read_all_once() != 0) {
       ERROR("Error: one or more plugin read callbacks failed.");
       exit_status = 1;
@@ -681,7 +695,7 @@ int main(int argc, char **argv) {
   }
 
 #if COLLECT_DAEMON
-  if (daemonize)
+  if (config.daemonize)
     pidfile_remove();
 #endif /* COLLECT_DAEMON */
 
index 01d484e..0558aa4 100644 (file)
 #include <sys/param.h>
 #endif
 
-#if HAVE_KSTAT_H
-#include <kstat.h>
-#endif
-
 #ifndef PACKAGE_NAME
 #define PACKAGE_NAME "collectd"
 #endif
 #define GAUGE_FORMAT "%.15g"
 #endif
 
-/* Type for time as used by "utils_time.h" */
-typedef uint64_t cdtime_t;
-
-extern char hostname_g[];
-extern cdtime_t interval_g;
-extern int timeout_g;
+#include "globals.h"
 
 #endif /* COLLECTD_H */
index 0d295c1..f5086ae 100644 (file)
@@ -461,9 +461,9 @@ static int cf_ci_replace_child(oconfig_item_t *dst, oconfig_item_t *src,
     return 0;
   }
 
-  temp =
-      realloc(dst->children, sizeof(oconfig_item_t) *
-                                 (dst->children_num + src->children_num - 1));
+  temp = realloc(dst->children,
+                 sizeof(oconfig_item_t) *
+                     (dst->children_num + src->children_num - 1));
   if (temp == NULL) {
     ERROR("configfile: realloc failed.");
     return -1;
@@ -502,8 +502,9 @@ static int cf_ci_append_children(oconfig_item_t *dst, oconfig_item_t *src) {
   if ((src == NULL) || (src->children_num == 0))
     return 0;
 
-  temp = realloc(dst->children, sizeof(oconfig_item_t) *
-                                    (dst->children_num + src->children_num));
+  temp =
+      realloc(dst->children,
+              sizeof(oconfig_item_t) * (dst->children_num + src->children_num));
   if (temp == NULL) {
     ERROR("configfile: realloc failed.");
     return -1;
@@ -874,7 +875,8 @@ const char *global_option_get(const char *option) {
     return NULL;
   }
 
-  return (cf_global_options[i].value != NULL) ? cf_global_options[i].value : cf_global_options[i].def;
+  return (cf_global_options[i].value != NULL) ? cf_global_options[i].value
+                                              : cf_global_options[i].def;
 } /* char *global_option_get */
 
 long global_option_get_long(const char *option, long default_value) {
@@ -1031,6 +1033,7 @@ int cf_read(const char *filename) {
   }
 
   return ret;
+
 } /* int cf_read */
 
 /* Assures the config option is a string, duplicates it and returns the copy in
diff --git a/src/daemon/globals.c b/src/daemon/globals.c
new file mode 100644 (file)
index 0000000..5c6749f
--- /dev/null
@@ -0,0 +1,48 @@
+/**
+ * collectd - src/globals.c
+ * Copyright (C) 2017  Google LLC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ **/
+
+#include "common.h"
+#include "globals.h"
+
+#if HAVE_KSTAT_H
+#include <kstat.h>
+#endif
+
+/*
+ * Global variables
+ */
+char *hostname_g;
+cdtime_t interval_g;
+int timeout_g;
+#if HAVE_KSTAT_H
+kstat_ctl_t *kc;
+#endif
+
+void hostname_set(char const *hostname) {
+  char *h = strdup(hostname);
+  if (h == NULL)
+    return;
+
+  sfree(hostname_g);
+  hostname_g = h;
+}
diff --git a/src/daemon/globals.h b/src/daemon/globals.h
new file mode 100644 (file)
index 0000000..bc11d6b
--- /dev/null
@@ -0,0 +1,43 @@
+/**
+ * collectd - src/globals.h
+ * Copyright (C) 2017  Google LLC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ **/
+
+#ifndef GLOBALS_H
+#define GLOBALS_H
+
+#include <inttypes.h>
+
+#ifndef DATA_MAX_NAME_LEN
+#define DATA_MAX_NAME_LEN 128
+#endif
+
+/* Type for time as used by "utils_time.h" */
+typedef uint64_t cdtime_t;
+
+/* hostname_set updates hostname_g */
+void hostname_set(char const *hostname);
+
+extern char *hostname_g;
+extern cdtime_t interval_g;
+extern int pidfile_from_cli;
+extern int timeout_g;
+#endif /* GLOBALS_H */
index 4f877e0..a9ee72d 100644 (file)
 
 #include <pthread.h>
 
-#ifndef DATA_MAX_NAME_LEN
-#define DATA_MAX_NAME_LEN 128
-#endif
-
 #define DS_TYPE_COUNTER 0
 #define DS_TYPE_GAUGE 1
 #define DS_TYPE_DERIVE 2
 #define DS_TYPE_ABSOLUTE 3
 
 #define DS_TYPE_TO_STRING(t)                                                   \
-  (t == DS_TYPE_COUNTER) ? "counter" : (t == DS_TYPE_GAUGE)                    \
-                                           ? "gauge"                           \
-                                           : (t == DS_TYPE_DERIVE)             \
-                                                 ? "derive"                    \
-                                                 : (t == DS_TYPE_ABSOLUTE)     \
-                                                       ? "absolute"            \
-                                                       : "unknown"
+  (t == DS_TYPE_COUNTER)                                                       \
+      ? "counter"                                                              \
+      : (t == DS_TYPE_GAUGE)                                                   \
+            ? "gauge"                                                          \
+            : (t == DS_TYPE_DERIVE)                                            \
+                  ? "derive"                                                   \
+                  : (t == DS_TYPE_ABSOLUTE) ? "absolute" : "unknown"
 
 #ifndef LOG_ERR
 #define LOG_ERR 3
index ca98539..6df4c15 100644 (file)
@@ -30,7 +30,7 @@
 kstat_ctl_t *kc = NULL;
 #endif /* HAVE_LIBKSTAT */
 
-char hostname_g[] = "example.com";
+char *hostname_g = "example.com";
 
 void plugin_set_dir(const char *dir) { /* nop */
 }
index d2a00bf..671d1f3 100644 (file)
@@ -261,12 +261,6 @@ struct {
                  {"Collectd::NOTIF_WARNING", NOTIF_WARNING},
                  {"Collectd::NOTIF_OKAY", NOTIF_OKAY},
                  {"", 0}};
-
-struct {
-  char name[64];
-  char *var;
-} g_strings[] = {{"Collectd::hostname_g", hostname_g}, {"", NULL}};
-
 /*
  * Helper functions for data type conversion.
  */
@@ -2099,7 +2093,7 @@ static int perl_init(void) {
   /* Lock the base thread to avoid race conditions with c_ithread_create().
    * See https://github.com/collectd/collectd/issues/9 and
    *     https://github.com/collectd/collectd/issues/1706 for details.
-  */
+   */
   assert(aTHX == perl_threads->head->interp);
   pthread_mutex_lock(&perl_threads->mutex);
 
@@ -2190,7 +2184,7 @@ static void perl_log(int level, const char *msg, user_data_t *user_data) {
   /* Lock the base thread if this is not called from one of the read threads
    * to avoid race conditions with c_ithread_create(). See
    * https://github.com/collectd/collectd/issues/9 for details.
-  */
+   */
 
   if (aTHX == perl_threads->head->interp)
     pthread_mutex_lock(&perl_threads->mutex);
@@ -2349,14 +2343,25 @@ static int g_interval_set(pTHX_ SV *var, MAGIC *mg) {
   return 0;
 } /* static int g_interval_set (pTHX_ SV *, MAGIC *) */
 
-static MGVTBL g_pv_vtbl = {g_pv_get, g_pv_set, NULL, NULL, NULL, NULL, NULL
+static MGVTBL g_pv_vtbl = {g_pv_get,
+                           g_pv_set,
+                           NULL,
+                           NULL,
+                           NULL,
+                           NULL,
+                           NULL
 #if HAVE_PERL_STRUCT_MGVTBL_SVT_LOCAL
                            ,
                            NULL
 #endif
 };
-static MGVTBL g_interval_vtbl = {g_interval_get, g_interval_set, NULL, NULL,
-                                 NULL, NULL, NULL
+static MGVTBL g_interval_vtbl = {g_interval_get,
+                                 g_interval_set,
+                                 NULL,
+                                 NULL,
+                                 NULL,
+                                 NULL,
+                                 NULL
 #if HAVE_PERL_STRUCT_MGVTBL_SVT_LOCAL
                                  ,
                                  NULL
@@ -2390,6 +2395,11 @@ static void xs_init(pTHX) {
    * accessing any such variable (this is basically the same as using
    * tie() in Perl) */
   /* global strings */
+  struct {
+    char name[64];
+    char *var;
+  } g_strings[] = {{"Collectd::hostname_g", hostname_g}, {"", NULL}};
+
   for (int i = 0; '\0' != g_strings[i].name[0]; ++i) {
     tmp = get_sv(g_strings[i].name, 1);
     sv_magicext(tmp, NULL, PERL_MAGIC_ext, &g_pv_vtbl, g_strings[i].var, 0);