Fix a NULL pointer dereference during shutdown
[collectd.git] / src / plugin.c
index 547b5eb..942f8bf 100644 (file)
@@ -74,6 +74,7 @@ typedef struct write_queue_s write_queue_t;
 struct write_queue_s
 {
        value_list_t *vl;
+       plugin_ctx_t ctx;
        write_queue_t *next;
 };
 
@@ -629,6 +630,31 @@ static value_list_t *plugin_value_list_clone (value_list_t const *vl_orig) /* {{
                return (NULL);
        }
 
+       if (vl->time == 0)
+               vl->time = cdtime ();
+
+       /* Fill in the interval from the thread context, if it is zero. */
+       if (vl->interval == 0)
+       {
+               plugin_ctx_t ctx = plugin_get_ctx ();
+
+               if (ctx.interval != 0)
+                       vl->interval = ctx.interval;
+               else
+               {
+                       char name[6 * DATA_MAX_NAME_LEN];
+                       FORMAT_VL (name, sizeof (name), vl);
+                       ERROR ("plugin_value_list_clone: Unable to determine "
+                                       "interval from context for "
+                                       "value list \"%s\". "
+                                       "This indicates a broken plugin. "
+                                       "Please report this problem to the "
+                                       "collectd mailing list or at "
+                                       "<http://collectd.org/bugs/>.", name);
+                       vl->interval = cf_get_default_interval ();
+               }
+       }
+
        return (vl);
 } /* }}} value_list_t *plugin_value_list_clone */
 
@@ -648,6 +674,11 @@ static int plugin_write_enqueue (value_list_t const *vl) /* {{{ */
                return (ENOMEM);
        }
 
+       /* Store context of caller (read plugin); otherwise, it would not be
+        * available to the write plugins when actually dispatching the
+        * value-list later on. */
+       q->ctx = plugin_get_ctx ();
+
        pthread_mutex_lock (&write_lock);
 
        if (write_queue_tail == NULL)
@@ -690,6 +721,8 @@ static value_list_t *plugin_write_dequeue (void) /* {{{ */
 
        pthread_mutex_unlock (&write_lock);
 
+       (void) plugin_set_ctx (q->ctx);
+
        vl = q->vl;
        sfree (q);
        return (vl);
@@ -777,10 +810,12 @@ static void stop_write_threads (void) /* {{{ */
 
        pthread_mutex_lock (&write_lock);
        i = 0;
-       for (q = write_queue_head; q != NULL; q = q->next)
+       for (q = write_queue_head; q != NULL; )
        {
+               write_queue_t *q1 = q;
                plugin_value_list_free (q->vl);
-               sfree (q);
+               q = q->next;
+               sfree (q1);
                i++;
        }
        write_queue_head = NULL;
@@ -826,8 +861,6 @@ int plugin_load (const char *type, uint32_t flags)
        struct dirent *de;
        int status;
 
-       DEBUG ("type = %s", type);
-
        dir = plugin_get_dir ();
        ret = 1;
 
@@ -836,7 +869,7 @@ int plugin_load (const char *type, uint32_t flags)
        status = ssnprintf (typename, sizeof (typename), "%s.so", type);
        if ((status < 0) || ((size_t) status >= sizeof (typename)))
        {
-               WARNING ("snprintf: truncated: `%s.so'", type);
+               WARNING ("plugin_load: Filename too long: \"%s.so\"", type);
                return (-1);
        }
        typename_len = strlen (typename);
@@ -844,7 +877,7 @@ int plugin_load (const char *type, uint32_t flags)
        if ((dh = opendir (dir)) == NULL)
        {
                char errbuf[1024];
-               ERROR ("opendir (%s): %s", dir,
+               ERROR ("plugin_load: opendir (%s) failed: %s", dir,
                                sstrerror (errno, errbuf, sizeof (errbuf)));
                return (-1);
        }
@@ -858,25 +891,29 @@ int plugin_load (const char *type, uint32_t flags)
                                "%s/%s", dir, de->d_name);
                if ((status < 0) || ((size_t) status >= sizeof (filename)))
                {
-                       WARNING ("snprintf: truncated: `%s/%s'", dir, de->d_name);
+                       WARNING ("plugin_load: Filename too long: \"%s/%s\"",
+                                       dir, de->d_name);
                        continue;
                }
 
                if (lstat (filename, &statbuf) == -1)
                {
                        char errbuf[1024];
-                       WARNING ("stat %s: %s", filename,
+                       WARNING ("plugin_load: stat (\"%s\") failed: %s",
+                                       filename,
                                        sstrerror (errno, errbuf, sizeof (errbuf)));
                        continue;
                }
                else if (!S_ISREG (statbuf.st_mode))
                {
                        /* don't follow symlinks */
-                       WARNING ("stat %s: not a regular file", filename);
+                       WARNING ("plugin_load: %s is not a regular file.",
+                                       filename);
                        continue;
                }
 
-               if (plugin_load_file (filename, flags) == 0)
+               status = plugin_load_file (filename, flags);
+               if (status == 0)
                {
                        /* success */
                        ret = 0;
@@ -884,14 +921,16 @@ int plugin_load (const char *type, uint32_t flags)
                }
                else
                {
-                       fprintf (stderr, "Unable to load plugin %s.\n", type);
+                       ERROR ("plugin_load: Load plugin \"%s\" failed with "
+                                       "status %i.", type, status);
                }
        }
 
        closedir (dh);
 
-       if (filename[0] == '\0')
-               fprintf (stderr, "Could not find plugin %s.\n", type);
+       if (filename[0] == 0)
+               ERROR ("plugin_load: Could not find plugin \"%s\" in %s",
+                               type, dir);
 
        return (ret);
 }
@@ -1774,29 +1813,10 @@ static int plugin_dispatch_values_internal (value_list_t *vl)
                return (-1);
        }
 
-       if (vl->time == 0)
-               vl->time = cdtime ();
-
-       if (vl->interval <= 0)
-       {
-               plugin_ctx_t ctx = plugin_get_ctx ();
-
-               if (ctx.interval != 0)
-                       vl->interval = ctx.interval;
-               else
-               {
-                       char name[6 * DATA_MAX_NAME_LEN];
-                       FORMAT_VL (name, sizeof (name), vl);
-                       ERROR ("plugin_dispatch_values: Unable to determine "
-                                       "interval from context for "
-                                       "value list \"%s\". "
-                                       "This indicates a broken plugin. "
-                                       "Please report this problem to the "
-                                       "collectd mailing list or at "
-                                       "<http://collectd.org/bugs/>.", name);
-                       vl->interval = cf_get_default_interval ();
-               }
-       }
+       /* Assured by plugin_value_list_clone(). The time is determined at
+        * _enqueue_ time. */
+       assert (vl->time != 0);
+       assert (vl->interval != 0);
 
        DEBUG ("plugin_dispatch_values: time = %.3f; interval = %.3f; "
                        "host = %s; "