From ef2638d00a1bcc1d4ef09a95d93fec10f64e45b4 Mon Sep 17 00:00:00 2001 From: Luca Deri Date: Wed, 28 Mar 2018 08:17:33 +0200 Subject: [PATCH] Fixed file descriptor leak --- include/TimeSeriesExporter.h | 2 +- src/TimeSeriesExporter.cpp | 51 ++++++++++++++++++++++-------------- src/Utils.cpp | 4 ++- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/include/TimeSeriesExporter.h b/include/TimeSeriesExporter.h index 1680616c55..5dfed6d3c5 100644 --- a/include/TimeSeriesExporter.h +++ b/include/TimeSeriesExporter.h @@ -27,7 +27,7 @@ class TimeSeriesExporter { private: time_t flushTime; - FILE *fd; + int fd; char fname[PATH_MAX]; NetworkInterface *iface; char *url; diff --git a/src/TimeSeriesExporter.cpp b/src/TimeSeriesExporter.cpp index 18ee50d876..af62979dbd 100644 --- a/src/TimeSeriesExporter.cpp +++ b/src/TimeSeriesExporter.cpp @@ -29,7 +29,7 @@ redis-cli set "ntopng.prefs.ts_post_data_url" "http://localhost:8086/write?db=ntopng" [InfluxDB] */ TimeSeriesExporter::TimeSeriesExporter(NetworkInterface *_if, char *_url) { - fd = NULL, iface = _if, url = strdup(_url), num_cached_entries = 0; + fd = -1, iface = _if, url = strdup(_url), num_cached_entries = 0; ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] Exporting TS data to %s", iface->get_name(), url); } @@ -45,11 +45,15 @@ TimeSeriesExporter::~TimeSeriesExporter() { void TimeSeriesExporter::createDump() { strcpy(fname, "/tmp/TimeSeriesExporter_XXXXXX"); - mkstemp(fname); + fd = mkstemp(fname); - fd = fopen(fname, "w"); - ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] Dumping TS data onto tmp file %s", - iface->get_name(), fname); + if(fd == -1) + ntop->getTrace()->traceEvent(TRACE_ERROR, "[%s] Unable to dump TS data onto %s: %s", + iface->get_name(), fname, strerror(errno)); + else + ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] Dumping TS data onto %s", + iface->get_name(), fname); + flushTime = time(NULL) + CONST_TS_FLUSH_TIME, num_cached_entries = 0; } @@ -57,15 +61,22 @@ void TimeSeriesExporter::createDump() { void TimeSeriesExporter::exportData(char *data) { m.lock(__FILE__, __LINE__); - - if(!fd) createDump(); - if(fd) { - fwrite(data, strlen(data), 1, fd), num_cached_entries++; - ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] %s", - iface->get_name(), data); + if(fd == -1) + createDump(); + + if(fd != -1) { + ssize_t exp = strlen(data); + ssize_t l = write(fd, data, exp); + + num_cached_entries++; + if(l == exp) + ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] %s", iface->get_name(), data); + else + ntop->getTrace()->traceEvent(TRACE_ERROR, "[%s] Unable to append '%s' [written: %u][expected: %u]", + iface->get_name(), data, exp, l); } - + m.unlock(__FILE__, __LINE__); if(time(NULL) > flushTime) @@ -75,13 +86,15 @@ void TimeSeriesExporter::exportData(char *data) { /* ******************************************************* */ void TimeSeriesExporter::flush() { - if(!fd) return; - m.lock(__FILE__, __LINE__); - fclose(fd); - fd = NULL; - ntop->getRedis()->rpush(CONST_TS_FILE_QUEUE, fname, 0); - ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] Queueing tmp file %s [%u entries]", - iface->get_name(), fname, num_cached_entries); + + if(fd != -1) { + close(fd); + fd = -1; + ntop->getRedis()->rpush(CONST_TS_FILE_QUEUE, fname, 0); + ntop->getTrace()->traceEvent(TRACE_INFO, "[%s] Queueing TS file %s [%u entries]", + iface->get_name(), fname, num_cached_entries); + } + m.unlock(__FILE__, __LINE__); } diff --git a/src/Utils.cpp b/src/Utils.cpp index 58a29b6ff9..07b6fd20e1 100755 --- a/src/Utils.cpp +++ b/src/Utils.cpp @@ -1072,7 +1072,7 @@ bool Utils::postHTTPTextFile(char *username, char *password, char *url, size_t file_len; FILE *fd = fopen(path, "r"); - if((fd== NULL) || (stat(path, &file_info) != 0)) + if((fd == NULL) || (stat(path, &file_info) != 0)) return(false); else file_len = (size_t)file_info.st_size; @@ -1120,6 +1120,8 @@ bool Utils::postHTTPTextFile(char *username, char *password, char *url, ntop->getTrace()->traceEvent(TRACE_INFO, "Posted JSON to %s", url); readCurlStats(curl, stats, NULL); } + + fclose(fd); /* always cleanup */ curl_slist_free_all(headers);