[checkmk-commits] Use std::string instead of strdup/free, removing a cruel owneship hack on the way.

Module: check_mk Branch: master Commit: 745c1d1eaf6b7bfe3195193a39607e9f35e8ac7d URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=745c1d1eaf6b7b… Author: Sven Panne &lt;sp(a)mathias-kettner.de&gt; Date: Mon Jul 31 09:54:20 2017 +0200 Use std::string instead of strdup/free, removing a cruel owneship hack on the way. Change-Id: I8ed033750b2afa5ecfcbe3f51a6f13b6ee688730 --- livestatus/src/HostServiceState.cc | 8 -------- livestatus/src/HostServiceState.h | 8 +------- livestatus/src/TableStateHistory.cc | 34 +++++++--------------------------- 3 files changed, 8 insertions(+), 42 deletions(-) diff --git a/livestatus/src/HostServiceState.cc b/livestatus/src/HostServiceState.cc index 5a1f721..33f0189 100644 --- a/livestatus/src/HostServiceState.cc +++ b/livestatus/src/HostServiceState.cc @@ -23,7 +23,6 @@ // Boston, MA 02110-1301 USA. #include "HostServiceState.h" -#include <cstdlib> HostServiceState::HostServiceState() : _is_host(false) @@ -53,16 +52,9 @@ HostServiceState::HostServiceState() , _may_no_longer_exist(false) , _has_vanished(false) , _last_known_time(0) - , _log_output(nullptr) , _host(nullptr) , _service(nullptr) {} -HostServiceState::~HostServiceState() { - if (_log_output != nullptr) { - free(_log_output); - } -} - #ifdef CMC void HostServiceState::computePerStateDurations() { _duration_state_UNMONITORED = 0; diff --git a/livestatus/src/HostServiceState.h b/livestatus/src/HostServiceState.h index c7bf6a9..24852cb 100644 --- a/livestatus/src/HostServiceState.h +++ b/livestatus/src/HostServiceState.h @@ -86,12 +86,7 @@ public: time_t _last_known_time; std::string _debug_info; - - // NOTE: _log_output is the *only* pointer in this class to an object we - // own, all other pointers are to foreign objects. This ownership is - // unfortunate and complicates things quite a lot, see the corresponding - // TODO in TableStateHistory::updateHostServiceState. - char *_log_output; + std::string _log_output; // maybe "": -> no period known, we assume "always" std::string _notification_period; @@ -103,7 +98,6 @@ public: std::string _service_description; // Fallback if service no longer exists HostServiceState(); - ~HostServiceState(); #ifdef CMC void computePerStateDurations(); #endif diff --git a/livestatus/src/TableStateHistory.cc b/livestatus/src/TableStateHistory.cc index b14949e..ba28f6e 100644 --- a/livestatus/src/TableStateHistory.cc +++ b/livestatus/src/TableStateHistory.cc @@ -162,7 +162,7 @@ TableStateHistory::TableStateHistory(MonitoringCore *mc, LogCache *log_cache) "service_description", "Description of the service", DANGEROUS_OFFSETOF(HostServiceState, _service_description), -1, -1, -1)); - addColumn(make_unique<OffsetStringColumn>( + addColumn(make_unique<OffsetSStringColumn>( "log_output", "Logfile output relevant for this state", DANGEROUS_OFFSETOF(HostServiceState, _log_output), -1, -1, -1)); addColumn(make_unique<OffsetIntColumn>( @@ -662,10 +662,7 @@ void TableStateHistory::answerQuery(Query *query) { hst->_state = -1; hst->_until = hst->_time; hst->_debug_info = "UNMONITORED"; - if (hst->_log_output != nullptr) { - free(hst->_log_output); - } - hst->_log_output = nullptr; + hst->_log_output = ""; } hst->_time = _until - 1; @@ -710,10 +707,7 @@ int TableStateHistory::updateHostServiceState(Query *query, hs_state->_in_notification_period = 0; hs_state->_in_service_period = 0; hs_state->_is_flapping = 0; - if (hs_state->_log_output != nullptr) { - free(hs_state->_log_output); - } - hs_state->_log_output = nullptr; + hs_state->_log_output = ""; // Apply latest notification period information and set the host_state // to unmonitored @@ -877,28 +871,14 @@ int TableStateHistory::updateHostServiceState(Query *query, } if (entry->_type != LogEntryType::timeperiod_transition) { - if (hs_state->_log_output != nullptr) { - free(hs_state->_log_output); - } - if ((entry->_type == LogEntryType::state_host_initial || entry->_type == LogEntryType::state_service_initial) && (entry->_check_output != nullptr && - (strcmp(entry->_check_output, "(null)") == 0))) { - hs_state->_log_output = nullptr; - + strcmp(entry->_check_output, "(null)") == 0)) { + hs_state->_log_output = ""; } else { - // TODO(sp): Do we really need to strdup? How are the lifetimes of - // entry and hs_state related? This is highly unclear. This strdup - // complicates things like hell, because HostServiceState owns - // _log_output because of it. If this is really needed (hopefully - // not), we should better change the type from a naked pointer to a - // unique_ptr, but this would mean introducing yet another Column - // subclass, because the Column framework is not flexible at all - // regarding the types it handles. - hs_state->_log_output = entry->_check_output != nullptr - ? strdup(entry->_check_output) - : nullptr; + hs_state->_log_output = + entry->_check_output == nullptr ? "" : entry->_check_output; } }