Module: check_mk
Branch: master
Commit: d24890d35015dc5fd2c0a37c0fb111f8e2548009
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=d24890d35015dc…
Author: Sven Panne <sp(a)mathias-kettner.de>
Date: Wed Jan 20 11:28:11 2016 +0100
Remove a few useless casts. Added TODOs about ownership/lifetime issues.
---
livestatus/src/HostServiceState.h | 8 ++++++--
livestatus/src/TableStateHistory.cc | 26 +++++++++++++++++---------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/livestatus/src/HostServiceState.h b/livestatus/src/HostServiceState.h
index 562080f..3a57d4a 100644
--- a/livestatus/src/HostServiceState.h
+++ b/livestatus/src/HostServiceState.h
@@ -79,9 +79,13 @@ struct HostServiceState {
const char *_debug_info;
- // Pointer to dynamically allocated strings (strdup) that live here.
- // These pointers are 0, if there is no output (e.g. downtime)
+
+ // 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;
+
const char *_notification_period; // may be "": -> no period known, we
assume "always"
const char *_service_period; // may be "": -> no period known, we
assume "always"
host *_host;
diff --git a/livestatus/src/TableStateHistory.cc b/livestatus/src/TableStateHistory.cc
index cf72fe2..093030f 100644
--- a/livestatus/src/TableStateHistory.cc
+++ b/livestatus/src/TableStateHistory.cc
@@ -437,40 +437,40 @@ void TableStateHistory::answerQuery(Query *query)
// If this host/service is no longer availabe in nagios -> set to
""
if (state->_service != 0)
#ifdef CMC
- state->_notification_period = (char
*)state->_service->notificationPeriod()->name();
+ state->_notification_period =
state->_service->notificationPeriod()->name();
#else
state->_notification_period =
state->_service->notification_period;
#endif
else if (state->_host != 0)
#ifdef CMC
- state->_notification_period = (char
*)state->_host->notificationPeriod()->name();
+ state->_notification_period =
state->_host->notificationPeriod()->name();
#else
state->_notification_period =
state->_host->notification_period;
#endif
else
- state->_notification_period = (char *)"";
+ state->_notification_period = "";
// If for some reason the notification period is missing set a default
if (state->_notification_period == NULL) {
- state->_notification_period = (char *)"";
+ state->_notification_period = "";
}
// Same for service period. For Nagios this is a bit different, since
this
// is no native field but just a custom variable
if (state->_service != 0)
#ifdef CMC
- state->_service_period = (char
*)state->_service->servicePeriod()->name();
+ state->_service_period =
state->_service->servicePeriod()->name();
#else
- state->_service_period = (char
*)getCustomVariable(state->_service->custom_variables, "SERVICE_PERIOD");
+ state->_service_period =
getCustomVariable(state->_service->custom_variables, "SERVICE_PERIOD");
#endif
else if (state->_host != 0)
#ifdef CMC
- state->_service_period = (char
*)state->_host->servicePeriod()->name();
+ state->_service_period =
state->_host->servicePeriod()->name();
#else
- state->_service_period = (char
*)getCustomVariable(state->_host->custom_variables, "SERVICE_PERIOD");
+ state->_service_period =
getCustomVariable(state->_host->custom_variables, "SERVICE_PERIOD");
#endif
else
- state->_service_period = (char *)"";
+ state->_service_period = "";
// Determine initial in_notification_period status
@@ -785,6 +785,14 @@ int TableStateHistory::updateHostServiceState(Query *query, const
LogEntry *entr
hs_state->_log_output = 0;
else
+ // TODO: 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 ?
strdup(entry->_check_output) : 0;
}