fixes identified by coverity

diff --git a/src/plugin_tc.c b/src/plugin_tc.c
index 35763d316aceb6e05ed556266011d16abb372c17..7dcfedb33e56d6db1e0a60bc55b8b39fdef44360 100644 (file)
--- a/src/plugin_tc.c
+++ b/src/plugin_tc.c
@@ -1003,7 +1003,7 @@ void *tc_main(void *ptr) {
                 rrddim_set(stcpu, "system", thread.ru_stime.tv_sec * 1000000ULL + thread.ru_stime.tv_usec);
                 rrdset_done(stcpu);
 
-                if(unlikely(!sttime)) stcpu = rrdset_find_localhost("netdata.plugin_tc_time");
+                if(unlikely(!sttime)) sttime = rrdset_find_localhost("netdata.plugin_tc_time");
                 if(unlikely(!sttime)) {
                     sttime = rrdset_create_localhost("netdata", "plugin_tc_time", NULL, "tc.helper", NULL
                                                      , "NetData TC script execution", "milliseconds/run", 135001

diff --git a/src/rrdcalctemplate.c b/src/rrdcalctemplate.c
index 1deec43a5ca2e04bc001c4e799225d033182d15d..2c5e2bd1616b1fc290e13022819cf98d59b3c4f2 100644 (file)
--- a/src/rrdcalctemplate.c
+++ b/src/rrdcalctemplate.c
@@ -23,6 +23,8 @@ void rrdcalctemplate_link_matching(RRDSET *st) {
 }
 
 inline void rrdcalctemplate_free(RRDHOST *host, RRDCALCTEMPLATE *rt) {
+    if(unlikely(!rt)) return;
+
     debug(D_HEALTH, "Health removing template '%s' of host '%s'", rt->name, host->hostname);
 
     if(host->templates == rt) {

diff --git a/src/rrdhost.c b/src/rrdhost.c
index 7002c4823472c7d85c97a3e65b7bfad0796e8333..c5bc6bd778964c3d565a482fcb6ce7d91033b96b 100644 (file)
--- a/src/rrdhost.c
+++ b/src/rrdhost.c
@@ -409,6 +409,10 @@ void rrdhost_free(RRDHOST *host) {
     info("Freeing all memory for host '%s'...", host->hostname);
 
     rrd_check_wrlock();     // make sure the RRDs are write locked
+
+    // stop a possibly running thread
+    rrdpush_sender_thread_stop(host);
+
     rrdhost_wrlock(host);   // lock this RRDHOST
 
     // ------------------------------------------------------------------------
@@ -447,8 +451,6 @@ void rrdhost_free(RRDHOST *host) {
     // ------------------------------------------------------------------------
     // free it
 
-    rrdpush_sender_thread_stop(host);
-
     freez(host->os);
     freez(host->cache_dir);
     freez(host->varlib_dir);

diff --git a/src/rrdpush.c b/src/rrdpush.c
index 06695c6f207e8a36e9f01019e22e4feb60fe50e2..07fcfead54001561c010491f83f46cb4e2cf555d 100644 (file)
--- a/src/rrdpush.c
+++ b/src/rrdpush.c
@@ -185,31 +185,18 @@ static void rrdpush_sender_thread_reset_all_charts(RRDHOST *host) {
 
 static inline void rrdpush_sender_thread_data_flush(RRDHOST *host) {
     rrdpush_lock(host);
+
     if(buffer_strlen(host->rrdpush_buffer))
         error("STREAM %s [send]: discarding %zu bytes of metrics already in the buffer.", host->hostname, buffer_strlen(host->rrdpush_buffer));
 
     buffer_flush(host->rrdpush_buffer);
-    rrdpush_sender_thread_reset_all_charts(host);
-    rrdpush_unlock(host);
-}
-
-static inline void rrdpush_sender_thread_lock(RRDHOST *host) {
-    if(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0)
-        error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname);
 
-    rrdpush_lock(host);
-}
-
-static inline void rrdpush_sender_thread_unlock(RRDHOST *host) {
-    if(pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0)
-        error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname);
+    rrdpush_sender_thread_reset_all_charts(host);
 
     rrdpush_unlock(host);
 }
 
-static void rrdpush_sender_thread_cleanup(RRDHOST *host) {
-    rrdpush_lock(host);
-
+static void rrdpush_sender_thread_cleanup_locked_all(RRDHOST *host) {
     host->rrdpush_connected = 0;
 
     if(host->rrdpush_socket != -1) {
@@ -232,18 +219,20 @@ static void rrdpush_sender_thread_cleanup(RRDHOST *host) {
     host->rrdpush_buffer = NULL;
 
     host->rrdpush_spawn = 0;
-
-    rrdpush_unlock(host);
 }
 
 void rrdpush_sender_thread_stop(RRDHOST *host) {
-    rrdhost_check_wrlock(host);
+    rrdpush_lock(host);
+    rrdhost_wrlock(host);
 
     if(host->rrdpush_spawn) {
         info("STREAM %s [send]: stopping sending thread...", host->hostname);
         pthread_cancel(host->rrdpush_thread);
-        rrdpush_sender_thread_cleanup(host);
+        rrdpush_sender_thread_cleanup_locked_all(host);
     }
+
+    rrdhost_unlock(host);
+    rrdpush_unlock(host);
 }
 
 void *rrdpush_sender_thread(void *ptr) {
@@ -398,7 +387,19 @@ void *rrdpush_sender_thread(void *ptr) {
         }
 
         if(ofd->revents & POLLOUT && begin < buffer_strlen(host->rrdpush_buffer)) {
-            rrdpush_sender_thread_lock(host);
+
+            // BEGIN RRDPUSH LOCKED SESSION
+
+            // during this session, data collectors
+            // will not be able to append data to our buffer
+            // but the socket is in non-blocking mode
+            // so, we will not block at send()
+
+            if(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0)
+                error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname);
+
+            rrdpush_lock(host);
+
             ssize_t ret = send(host->rrdpush_socket, &host->rrdpush_buffer->buffer[begin], buffer_strlen(host->rrdpush_buffer) - begin, MSG_DONTWAIT);
             if(ret == -1) {
                 if(errno != EAGAIN && errno != EINTR) {
@@ -412,11 +413,19 @@ void *rrdpush_sender_thread(void *ptr) {
                 sent_bytes += ret;
                 begin += ret;
                 if(begin == buffer_strlen(host->rrdpush_buffer)) {
+                    // we send it all
+
                     buffer_flush(host->rrdpush_buffer);
                     begin = 0;
                 }
             }
-            rrdpush_sender_thread_unlock(host);
+
+            rrdpush_unlock(host);
+
+            if(pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0)
+                error("STREAM %s [send]: cannot set pthread cancel state to ENABLE.", host->hostname);
+
+            // END RRDPUSH LOCKED SESSION
         }
 
         // protection from overflow
@@ -433,7 +442,11 @@ void *rrdpush_sender_thread(void *ptr) {
 cleanup:
     debug(D_WEB_CLIENT, "STREAM %s [send]: sending thread exits.", host->hostname);
 
-    rrdpush_sender_thread_cleanup(host);
+    rrdpush_lock(host);
+    rrdhost_wrlock(host);
+    rrdpush_sender_thread_cleanup_locked_all(host);
+    rrdhost_unlock(host);
+    rrdpush_unlock(host);
 
     pthread_exit(NULL);
     return NULL;
@@ -534,6 +547,7 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m
     info("STREAM %s [receive from [%s]:%s]: initializing communication...", host->hostname, client_ip, client_port);
     if(send_timeout(fd, START_STREAMING_PROMPT, strlen(START_STREAMING_PROMPT), 0, 60) != strlen(START_STREAMING_PROMPT)) {
         error("STREAM %s [receive from [%s]:%s]: cannot send ready command.", host->hostname, client_ip, client_port);
+        close(fd);
         return 0;
     }
 
@@ -545,6 +559,7 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m
     FILE *fp = fdopen(fd, "r");
     if(!fp) {
         error("STREAM %s [receive from [%s]:%s]: failed to get a FILE for FD %d.", host->hostname, client_ip, client_port, fd);
+        close(fd);
         return 0;
     }
 
@@ -566,11 +581,11 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m
             host->health_enabled = 0;
 
         host->senders_disconnected_time = now_realtime_sec();
-
-        rrdpush_sender_thread_stop(host);
     }
     rrdhost_unlock(host);
 
+    rrdpush_sender_thread_stop(host);
+
     // cleanup
     fclose(fp);
 
@@ -602,7 +617,6 @@ void *rrdpush_receiver_thread(void *ptr) {
     rrdpush_receive(rpt->fd, rpt->key, rpt->hostname, rpt->machine_guid, rpt->os, rpt->update_every, rpt->client_ip, rpt->client_port);
     info("STREAM %s [receive from [%s]:%s]: receive thread ended (task id %d)", rpt->hostname, rpt->client_ip, rpt->client_port, gettid());
 
-    close(rpt->fd);
     freez(rpt->key);
     freez(rpt->hostname);
     freez(rpt->machine_guid);
@@ -616,13 +630,19 @@ void *rrdpush_receiver_thread(void *ptr) {
 }
 
 void rrdpush_sender_thread_spawn(RRDHOST *host) {
-    if(pthread_create(&host->rrdpush_thread, NULL, rrdpush_sender_thread, (void *)host))
-        error("STREAM %s [send]: failed to create new thread for client.", host->hostname);
+    rrdhost_wrlock(host);
+
+    if(!host->rrdpush_spawn) {
+        if(pthread_create(&host->rrdpush_thread, NULL, rrdpush_sender_thread, (void *) host))
+            error("STREAM %s [send]: failed to create new thread for client.", host->hostname);
 
-    else if(pthread_detach(host->rrdpush_thread))
-        error("STREAM %s [send]: cannot request detach newly created thread.", host->hostname);
+        else if(pthread_detach(host->rrdpush_thread))
+            error("STREAM %s [send]: cannot request detach newly created thread.", host->hostname);
 
-    host->rrdpush_spawn = 1;
+        host->rrdpush_spawn = 1;
+    }
+
+    rrdhost_unlock(host);
 }
 
 int rrdpush_receiver_thread_spawn(RRDHOST *host, struct web_client *w, char *url) {
@@ -630,7 +650,7 @@ int rrdpush_receiver_thread_spawn(RRDHOST *host, struct web_client *w, char *url
 
     info("STREAM [receive from [%s]:%s]: new client connection.", w->client_ip, w->client_port);
 
-    char *key = NULL, *hostname = NULL, *machine_guid = NULL, *os = NULL;
+    char *key = NULL, *hostname = NULL, *machine_guid = NULL, *os = "unknown";
     int update_every = default_rrd_update_every;
     char buf[GUID_LEN + 1];
 

diff --git a/src/sys_fs_cgroup.c b/src/sys_fs_cgroup.c
index 248f3d626fd5838ef1e396d8b54d36c41abbeca7..088495a6dea3307696f8622d169ea15b98129867 100644 (file)
--- a/src/sys_fs_cgroup.c
+++ b/src/sys_fs_cgroup.c
@@ -466,8 +466,8 @@ static inline void cgroup_read_cpuacct_usage(struct cpuacct_usage *ca) {
 
         unsigned long i = procfile_linewords(ff, 0);
         if(unlikely(i == 0)) {
-            return;
             ca->updated = 0;
+            return;
         }
 
         // we may have 1 more CPU reported
@@ -2113,7 +2113,7 @@ void update_cgroup_charts(int update_every) {
                 );
 
                 for(i = 0; i < cg->cpuacct_usage.cpus; i++) {
-                    snprintfz(id, CHART_TITLE_MAX, "cpu%u", i);
+                    snprintfz(id, RRD_ID_LENGTH_MAX, "cpu%u", i);
                     rrddim_add(cg->st_cpu_per_core, id, NULL, 100, 1000000000, RRD_ALGORITHM_INCREMENTAL);
                 }
             }
@@ -2121,7 +2121,7 @@ void update_cgroup_charts(int update_every) {
                 rrdset_next(cg->st_cpu_per_core);
 
             for(i = 0; i < cg->cpuacct_usage.cpus ;i++) {
-                snprintfz(id, CHART_TITLE_MAX, "cpu%u", i);
+                snprintfz(id, RRD_ID_LENGTH_MAX, "cpu%u", i);
                 rrddim_set(cg->st_cpu_per_core, id, cg->cpuacct_usage.cpu_percpu[i]);
             }
             rrdset_done(cg->st_cpu_per_core);