src/daemon/plugin.c: Address review comments.

diff --git a/configure.ac b/configure.ac
index 9c5df839f67b40e86374292a54f4e1b2d84a621b..49baf0a35f323613e880e439f37af733bdced802 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -214,7 +214,54 @@ AC_HEADER_SYS_WAIT
 AC_HEADER_DIRENT
 AC_HEADER_STDBOOL
 
-AC_CHECK_HEADERS(stdio.h errno.h math.h stdarg.h syslog.h fcntl.h signal.h assert.h sys/types.h sys/socket.h sys/select.h poll.h netdb.h arpa/inet.h sys/resource.h sys/param.h kstat.h regex.h sys/ioctl.h endian.h sys/isa_defs.h fnmatch.h libgen.h pthread_np.h)
+AC_CHECK_HEADERS([ \
+  arpa/inet.h \
+  assert.h \
+  ctype.h \
+  endian.h \
+  errno.h \
+  fcntl.h \
+  fnmatch.h \
+  fs_info.h \
+  fshelp.h \
+  grp.h \
+  kstat.h \
+  kvm.h \
+  libgen.h \
+  limits.h \
+  locale.h \
+  math.h \
+  mntent.h \
+  mnttab.h \
+  netdb.h \
+  paths.h \
+  poll.h \
+  pthread_np.h \
+  pwd.h \
+  regex.h \
+  signal.h \
+  stdarg.h \
+  stdio.h \
+  sys/fs_types.h \
+  sys/fstyp.h \
+  sys/ioctl.h \
+  sys/isa_defs.h \
+  sys/mntent.h \
+  sys/mnttab.h \
+  sys/param.h \
+  sys/resource.h \
+  sys/select.h \
+  sys/socket.h \
+  sys/statfs.h \
+  sys/statvfs.h \
+  sys/types.h \
+  sys/un.h \
+  sys/vfs.h \
+  sys/vfstab.h \
+  sys/vmmeter.h \
+  syslog.h \
+  wordexp.h \
+])
 
 # For entropy plugin on newer NetBSD
 AC_CHECK_HEADERS(sys/rndio.h, [], [],
@@ -658,31 +705,6 @@ AC_CHECK_HEADERS(linux/un.h, [], [],
 #endif
 ])
 
-AC_CHECK_HEADERS([ \
-  ctype.h \
-  fs_info.h \
-  fshelp.h \
-  grp.h \
-  kvm.h \
-  limits.h \
-  locale.h \
-  mntent.h \
-  mnttab.h \
-  paths.h \
-  pwd.h \
-  sys/fs_types.h \
-  sys/fstyp.h \
-  sys/mntent.h \
-  sys/mnttab.h \
-  sys/statfs.h \
-  sys/statvfs.h \
-  sys/un.h \
-  sys/vfs.h \
-  sys/vfstab.h \
-  sys/vmmeter.h \
-  wordexp.h \
-])
-
 # --enable-xfs {{{
 AC_ARG_ENABLE([xfs],
   [AS_HELP_STRING([--enable-xfs], [xfs support in df plugin @<:@default=yes@:>@])],
@@ -1666,7 +1688,7 @@ AC_CHECK_MEMBERS([kstat_io_t.nwritten, kstat_io_t.writes, kstat_io_t.nwrites, ks
 SAVE_LDFLAGS="$LDFLAGS"
 LDFLAGS="$LDFLAGS -lpthread"
 
-AC_MSG_CHECKING([if have pthread_setname_np])
+AC_MSG_CHECKING([for pthread_setname_np])
        have_pthread_setname_np="no"
        AC_LINK_IFELSE([AC_LANG_PROGRAM(
 [[
@@ -1684,7 +1706,7 @@ AC_MSG_CHECKING([if have pthread_setname_np])
 AC_MSG_RESULT([$have_pthread_setname_np])
 
 # check for pthread_set_name_np(3) (FreeBSD)
-AC_MSG_CHECKING([if have pthread_set_name_np])
+AC_MSG_CHECKING([for pthread_set_name_np])
        have_pthread_set_name_np="no"
        AC_LINK_IFELSE([AC_LANG_PROGRAM(
 [[

diff --git a/src/daemon/plugin.c b/src/daemon/plugin.c
index ac9ce5bd16df5a40dc89a4543c3ccd89b344510d..d54abeeae083fb32c4d53704acf475a115d66a02 100644 (file)
--- a/src/daemon/plugin.c
+++ b/src/daemon/plugin.c
@@ -123,7 +123,7 @@ static int             read_loop = 1;
 static pthread_mutex_t read_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t  read_cond = PTHREAD_COND_INITIALIZER;
 static pthread_t      *read_threads = NULL;
-static int             read_threads_num = 0;
+static size_t          read_threads_num = 0;
 static cdtime_t        max_read_interval = DEFAULT_MAX_READ_INTERVAL;
 
 static write_queue_t  *write_queue_head;
@@ -651,7 +651,38 @@ static void *plugin_read_thread (void __attribute__((unused)) *args)
        return ((void *) 0);
 } /* void *plugin_read_thread */
 
-static void start_read_threads (int num)
+#ifdef PTHREAD_MAX_NAMELEN_NP
+# define THREAD_NAME_MAX PTHREAD_MAX_NAMELEN_NP
+#else
+# define THREAD_NAME_MAX 16
+#endif
+
+static void set_thread_name(pthread_t tid, char const *name) {
+#if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SET_NAME_NP)
+
+       /* glibc limits the length of the name and fails if the passed string
+        * is too long, so we truncate it here. */
+       char n[THREAD_NAME_MAX];
+       if (strlen (name) >= THREAD_NAME_MAX)
+               WARNING("set_thread_name(\"%s\"): name too long", name);
+       sstrncpy (n, name, sizeof(n));
+
+#if defined(HAVE_PTHREAD_SETNAME_NP)
+       int status = pthread_setname_np (tid, n);
+       if (status != 0)
+       {
+               char errbuf[1024];
+               ERROR ("set_thread_name(\"%s\"): %s", n,
+                               sstrerror (status, errbuf, sizeof(errbuf)));
+       }
+#else /* if defined(HAVE_PTHREAD_SET_NAME_NP) */
+       pthread_set_name_np (tid, n);
+#endif
+
+#endif
+}
+
+static void start_read_threads (size_t num) /* {{{ */
 {
        if (read_threads != NULL)
                return;
@@ -664,39 +695,35 @@ static void start_read_threads (int num)
        }
 
        read_threads_num = 0;
-       for (int i = 0; i < num; i++)
+       for (size_t i = 0; i < num; i++)
        {
-               if (pthread_create (read_threads + read_threads_num, NULL,
-                                       plugin_read_thread, NULL) == 0)
-               {
-#if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SET_NAME_NP)
-                       char thread_name[32];
-                       ssnprintf(thread_name, sizeof (thread_name),
-                                       "plugin reader#%d", i);
-# if defined(HAVE_PTHREAD_SETNAME_NP)
-                       pthread_setname_np (*(read_threads + read_threads_num),
-                               thread_name);
-# elif defined(HAVE_PTHREAD_SET_NAME_NP)
-                       pthread_set_name_np (*(read_threads + read_threads_num),
-                               thread_name);
-# endif
-#endif
-                       read_threads_num++;
-               }
-               else
+               int status = pthread_create (read_threads + read_threads_num,
+                               /* attr = */ NULL,
+                               plugin_read_thread,
+                               /* arg = */ NULL);
+               if (status != 0)
                {
-                       ERROR ("plugin: start_read_threads: pthread_create failed.");
+                       char errbuf[1024];
+                       ERROR ("plugin: start_read_threads: pthread_create failed "
+                                       "with status %i (%s).", status,
+                                       sstrerror (status, errbuf, sizeof (errbuf)));
                        return;
                }
+
+               char name[THREAD_NAME_MAX];
+               ssnprintf (name, sizeof (name), "reader#%zu", read_threads_num);
+               set_thread_name (read_threads[read_threads_num], name);
+
+               read_threads_num++;
        } /* for (i) */
-} /* void start_read_threads */
+} /* }}} void start_read_threads */
 
 static void stop_read_threads (void)
 {
        if (read_threads == NULL)
                return;
 
-       INFO ("collectd: Stopping %i read threads.", read_threads_num);
+       INFO ("collectd: Stopping %zu read threads.", read_threads_num);
 
        pthread_mutex_lock (&read_lock);
        read_loop = 0;
@@ -704,7 +731,7 @@ static void stop_read_threads (void)
        pthread_cond_broadcast (&read_cond);
        pthread_mutex_unlock (&read_lock);
 
-       for (int i = 0; i < read_threads_num; i++)
+       for (size_t i = 0; i < read_threads_num; i++)
        {
                if (pthread_join (read_threads[i], NULL) != 0)
                {
@@ -892,9 +919,7 @@ static void start_write_threads (size_t num) /* {{{ */
        write_threads_num = 0;
        for (size_t i = 0; i < num; i++)
        {
-               int status;
-
-               status = pthread_create (write_threads + write_threads_num,
+               int status = pthread_create (write_threads + write_threads_num,
                                /* attr = */ NULL,
                                plugin_write_thread,
                                /* arg = */ NULL);
@@ -905,20 +930,13 @@ static void start_write_threads (size_t num) /* {{{ */
                                        "with status %i (%s).", status,
                                        sstrerror (status, errbuf, sizeof (errbuf)));
                        return;
-               } else {
-#if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SET_NAME_NP)
-                       char thread_name[16];
-                       sstrncpy (thread_name, "plugin writer", sizeof(thread_name));
-# if defined(HAVE_PTHREAD_SETNAME_NP)
-                       pthread_setname_np (*(write_threads + write_threads_num),
-                               thread_name);
-# elif defined(HAVE_PTHREAD_SET_NAME_NP)
-                       pthread_set_name_np (*(write_threads + write_threads_num),
-                               thread_name);
-# endif
-#endif
-                       write_threads_num++;
                }
+
+               char name[THREAD_NAME_MAX];
+               ssnprintf (name, sizeof (name), "writer#%zu", write_threads_num);
+               set_thread_name (write_threads[write_threads_num], name);
+
+               write_threads_num++;
        } /* for (i) */
 } /* }}} void start_write_threads */
 
@@ -1807,7 +1825,7 @@ int plugin_init_all (void)
                rt = global_option_get ("ReadThreads");
                num = atoi (rt);
                if (num != -1)
-                       start_read_threads ((num > 0) ? num : 5);
+                       start_read_threads ((num > 0) ? ((size_t) num) : 5);
        }
        return ret;
 } /* void plugin_init_all */
@@ -2819,35 +2837,30 @@ static void *plugin_thread_start (void *arg)
 } /* void *plugin_thread_start */
 
 int plugin_thread_create (pthread_t *thread, const pthread_attr_t *attr,
-               void *(*start_routine) (void *), void *arg, char *name)
+               void *(*start_routine) (void *), void *arg, char const *name)
 {
        plugin_thread_t *plugin_thread;
-       int ret;
 
        plugin_thread = malloc (sizeof (*plugin_thread));
        if (plugin_thread == NULL)
-               return -1;
+               return ENOMEM;
 
        plugin_thread->ctx           = plugin_get_ctx ();
        plugin_thread->start_routine = start_routine;
        plugin_thread->arg           = arg;
 
-       ret = pthread_create (thread, attr,
+       int ret = pthread_create (thread, attr,
                        plugin_thread_start, plugin_thread);
-
-       if (ret == 0 && name != NULL) {
-#if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SET_NAME_NP)
-               char thread_name[16];
-               sstrncpy (thread_name, name, sizeof(thread_name));
-# if defined(HAVE_PTHREAD_SETNAME_NP)
-               pthread_setname_np (*thread, thread_name);
-# elif defined(HAVE_PTHREAD_SET_NAME_NP)
-               pthread_set_name_np (*thread, thread_name);
-# endif
-#endif
+       if (ret != 0)
+       {
+               sfree (plugin_thread);
+               return ret;
        }
 
-       return ret;
+       if (name != NULL)
+               set_thread_name (*thread, name);
+
+       return 0;
 } /* int plugin_thread_create */
 
 /* vim: set sw=8 ts=8 noet fdm=marker : */

diff --git a/src/daemon/plugin.h b/src/daemon/plugin.h
index 0f4d267c75149ad8017df77012a72e7988a53348..9a9bf497607d0a7240e287421d95dc6fe8149735 100644 (file)
--- a/src/daemon/plugin.h
+++ b/src/daemon/plugin.h
@@ -465,7 +465,7 @@ cdtime_t plugin_get_interval (void);
  */
 
 int plugin_thread_create (pthread_t *thread, const pthread_attr_t *attr,
-               void *(*start_routine) (void *), void *arg, char *name);
+               void *(*start_routine) (void *), void *arg, char const *name);
 
 /*
  * Plugins need to implement this

diff --git a/src/network.c b/src/network.c
index 6b43ae75b47982609ffbeb702488c1d554e1e049..d7eb32c3444f0b24867b6b49bdda131918daee5f 100644 (file)
--- a/src/network.c
+++ b/src/network.c
@@ -3477,7 +3477,7 @@ static int network_init (void)
                status = plugin_thread_create (&dispatch_thread_id,
                                NULL /* no attributes */,
                                dispatch_thread,
-                               NULL /* no argument */, "network dispatch");
+                               NULL /* no argument */, "network disp");
                if (status != 0)
                {
                        char errbuf[1024];

diff --git a/src/unixsock.c b/src/unixsock.c
index 4a7652ab5a1f4d924902eb5229285db84b81d561..2df4e096e7a63b03c3dda1b89143d4e1c405a19d 100644 (file)
--- a/src/unixsock.c
+++ b/src/unixsock.c
@@ -378,7 +378,7 @@ static void *us_server_thread (void __attribute__((unused)) *arg)
                DEBUG ("Spawning child to handle connection on fd #%i", *remote_fd);
 
                status = plugin_thread_create (&th, &th_attr,
-                               us_handle_client, (void *) remote_fd, "unixsock client");
+                               us_handle_client, (void *) remote_fd, "unixsock conn");
                if (status != 0)
                {
                        char errbuf[1024];
@@ -459,7 +459,7 @@ static int us_init (void)
        loop = 1;
 
        status = plugin_thread_create (&listen_thread, NULL,
-                       us_server_thread, NULL, "unixsock listener");
+                       us_server_thread, NULL, "unixsock listen");
        if (status != 0)
        {
                char errbuf[1024];