[PATCH 1/4] core: rework dead code. [PATCH 2/4] core: Remove unused variable.
This is not a coverity reported error. I just pulled out the formatting cleanup from the real code changes in patch 4/4. [PATCH 3/4] host: Clean up indentation.
[PATCH 4/4] qmf: Don't use string after it has been destroyed.
Our packages are built with MH_SSL not defined. Coverity was complaining about dead code because of this. Rework a bit of code to remove a condition that can never be true when MH_SSL is not enabled.
Signed-off-by: Russell Bryant rbryant@redhat.com --- src/lib/mh_agent.cpp | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/lib/mh_agent.cpp b/src/lib/mh_agent.cpp index ef2d305..842b496 100644 --- a/src/lib/mh_agent.cpp +++ b/src/lib/mh_agent.cpp @@ -278,9 +278,12 @@ mh_parse_options(const char *proc_name, int argc, char **argv, qpid::types::Vari std::stringstream url; qpid::types::Variant::Map amqp_options;
+#ifdef MH_SSL const char *ssl_cert_db = NULL; const char *ssl_cert_name = NULL; const char *ssl_cert_password_file = NULL; +#endif /* MH_SSL */ + int lpc = 0;
amqp_options["reconnect"] = true; @@ -441,8 +444,11 @@ mh_parse_options(const char *proc_name, int argc, char **argv, qpid::types::Vari free(long_opts); #endif
+ options["protocol"] = "tcp"; + #ifdef MH_SSL if (ssl_cert_name && ssl_cert_db && ssl_cert_password_file) { + options["protocol"] = "ssl"; qpid::sys::ssl::SslOptions ssl_options; ssl_options.certDbPath = strdup(ssl_cert_db); ssl_options.certName = strdup(ssl_cert_name); @@ -455,12 +461,6 @@ mh_parse_options(const char *proc_name, int argc, char **argv, qpid::types::Vari } #endif
- if (ssl_cert_name && ssl_cert_db && ssl_cert_password_file) { - options["protocol"] = "ssl"; - } else { - options["protocol"] = "tcp"; - } - if(options["serverport"].asString().empty()) { options["serverport"] = string("49000"); }
Coverity pointed out a pointer that was set but never used. Just remove it.
Signed-off-by: Russell Bryant rbryant@redhat.com --- src/lib/mh_dbus_common.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/lib/mh_dbus_common.c b/src/lib/mh_dbus_common.c index 4e1359b..ad32017 100644 --- a/src/lib/mh_dbus_common.c +++ b/src/lib/mh_dbus_common.c @@ -283,8 +283,7 @@ matahari_class_init(MatahariClass *matahari_class) static void matahari_init(Matahari *matahari) { - MatahariPrivate *priv; - matahari->priv = priv = MATAHARI_GET_PRIVATE(matahari); + matahari->priv = MATAHARI_GET_PRIVATE(matahari); }
Dict *
Clean up mixed tab/space indentation.
Signed-off-by: Russell Bryant rbryant@redhat.com --- src/host/host-qmf.cpp | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/host/host-qmf.cpp b/src/host/host-qmf.cpp index 6eabbe6..5882a07 100644 --- a/src/host/host-qmf.cpp +++ b/src/host/host-qmf.cpp @@ -123,40 +123,40 @@ HostAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, } else if (methodName == "identify") { mh_host_identify(); } else if (methodName == "set_uuid") { - const char *uuid = NULL; - const char *lifetime = NULL; + const char *uuid = NULL; + const char *lifetime = NULL;
if(args.count("lifetime") > 0) { lifetime = args["lifetime"].asString().c_str(); }
if(args.count("uuid") > 0) { - int rc = 0; + int rc = 0; uuid = args["uuid"].asString().c_str(); - rc = mh_host_set_uuid(lifetime, uuid); - event.addReturnArgument("rc", rc); + rc = mh_host_set_uuid(lifetime, uuid); + event.addReturnArgument("rc", rc);
- /* Now refresh the properties in case they changed */ - _instance.setProperty("custom_uuid", mh_host_get_uuid("Custom")); - _instance.setProperty("uuid", mh_host_get_uuid("Filesystem")); + /* Now refresh the properties in case they changed */ + _instance.setProperty("custom_uuid", mh_host_get_uuid("Custom")); + _instance.setProperty("uuid", mh_host_get_uuid("Filesystem"));
} else { - session.raiseException(event, "No UUID supplied"); - goto bail; - } + session.raiseException(event, "No UUID supplied"); + goto bail; + }
} else if (methodName == "get_uuid") { - const char *uuid = NULL; - const char *lifetime = NULL; + const char *uuid = NULL; + const char *lifetime = NULL;
if(args.count("lifetime") > 0) { lifetime = args["lifetime"].asString().c_str(); }
- uuid = mh_host_get_uuid(lifetime); - if(uuid) { - event.addReturnArgument("uuid", uuid); - } + uuid = mh_host_get_uuid(lifetime); + if(uuid) { + event.addReturnArgument("uuid", uuid); + }
} else { session.raiseException(event, MH_NOT_IMPLEMENTED); @@ -179,7 +179,7 @@ HostAgent::setup(qmf::AgentSession session) _instance.setProperty("update_interval", DEFAULT_UPDATE_INTERVAL); _instance.setProperty("uuid", mh_host_get_uuid("Filesystem")); if(custom_uuid) { - _instance.setProperty("custom_uuid", custom_uuid); + _instance.setProperty("custom_uuid", custom_uuid); }
_instance.setProperty("hostname", mh_host_get_hostname());
A number of places grabbed the internal representation of a string and used it later after the string destructor had run. In general, the problems all looked like this:
const char *foo = args["foo"].asString().c_str();
This is bad. foo is not valid after this line of code since asString() is returning a temporary std::string. This code is grabbing an internal char * from the string and then it immediately gets destroyed. Any use of 'foo' after this is not valid.
Signed-off-by: Russell Bryant rbryant@redhat.com --- src/host/host-qmf.cpp | 27 ++++++++---------- src/service/service-qmf-console.cpp | 4 +- src/service/service-qmf.cpp | 49 ++++++++++++++++++----------------- 3 files changed, 39 insertions(+), 41 deletions(-)
diff --git a/src/host/host-qmf.cpp b/src/host/host-qmf.cpp index 5882a07..7235655 100644 --- a/src/host/host-qmf.cpp +++ b/src/host/host-qmf.cpp @@ -123,17 +123,14 @@ HostAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, } else if (methodName == "identify") { mh_host_identify(); } else if (methodName == "set_uuid") { - const char *uuid = NULL; - const char *lifetime = NULL; - - if(args.count("lifetime") > 0) { - lifetime = args["lifetime"].asString().c_str(); - } - - if(args.count("uuid") > 0) { + if (args.count("uuid")) { int rc = 0; - uuid = args["uuid"].asString().c_str(); - rc = mh_host_set_uuid(lifetime, uuid); + if (args.count("lifetime")) { + rc = mh_host_set_uuid(args["lifetime"].asString().c_str(), + args["uuid"].asString().c_str()); + } else { + rc = mh_host_set_uuid(NULL, args["uuid"].asString().c_str()); + } event.addReturnArgument("rc", rc);
/* Now refresh the properties in case they changed */ @@ -147,14 +144,14 @@ HostAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event,
} else if (methodName == "get_uuid") { const char *uuid = NULL; - const char *lifetime = NULL;
- if(args.count("lifetime") > 0) { - lifetime = args["lifetime"].asString().c_str(); + if (args.count("lifetime")) { + uuid = mh_host_get_uuid(args["lifetime"].asString().c_str()); + } else { + uuid = mh_host_get_uuid(NULL); }
- uuid = mh_host_get_uuid(lifetime); - if(uuid) { + if (uuid) { event.addReturnArgument("uuid", uuid); }
diff --git a/src/service/service-qmf-console.cpp b/src/service/service-qmf-console.cpp index 1bf3a14..0384081 100644 --- a/src/service/service-qmf-console.cpp +++ b/src/service/service-qmf-console.cpp @@ -125,7 +125,7 @@ int main(int argc, char** argv)
if (options.count("action")) { uint32_t lpc = 1; - const char *action = options["action"].asString().c_str(); + std::string action = options["action"].asString(); cout << "Building " << options["api-type"] << " options" << endl; if(options["api-type"] == "Services") { if(options["action"] == "list") { @@ -139,7 +139,7 @@ int main(int argc, char** argv) }
} else { - if(strstr(action, "list") != NULL) { + if(strstr(action.c_str(), "list") != NULL) { callOptions["standard"] = options["standard"].asString(); callOptions["provider"] = options["provider"].asString();
diff --git a/src/service/service-qmf.cpp b/src/service/service-qmf.cpp index 1166a85..a2df2e3 100644 --- a/src/service/service-qmf.cpp +++ b/src/service/service-qmf.cpp @@ -69,7 +69,7 @@ public: virtual int setup(qmf::AgentSession session); virtual gboolean invoke(qmf::AgentSession session, qmf::AgentEvent event, gpointer user_data); - void raiseEvent(svc_action_t *op, enum service_id service, const char *userdata); + void raiseEvent(svc_action_t *op, enum service_id service, const std::string &userdata); };
const char SrvAgent::SERVICES_NAME[] = "Services"; @@ -113,21 +113,21 @@ public: void AsyncCB::mh_async_callback(svc_action_t *op) { - const char *userdata = NULL; + std::string userdata; AsyncCB *cb_data = static_cast<AsyncCB *>(op->cb_data);
mh_trace("Completed: %s = %d", op->id, op->rc);
qpid::types::Variant::Map& args = cb_data->event.getArguments(); if (args.count("userdata") > 0) { - userdata = args["userdata"].asString().c_str(); + userdata = args["userdata"].asString(); }
if (cb_data->first_result) { if (cb_data->has_rc) { cb_data->event.addReturnArgument("rc", op->rc); } - if (userdata) { + if (userdata.length()) { cb_data->event.addReturnArgument("userdata", userdata); } cb_data->session.methodSuccess(cb_data->event); @@ -178,7 +178,7 @@ main(int argc, char **argv) }
void -SrvAgent::raiseEvent(svc_action_t *op, enum service_id service, const char *userdata) +SrvAgent::raiseEvent(svc_action_t *op, enum service_id service, const std::string &userdata) { uint64_t timestamp = 0L; qmf::Data event; @@ -210,7 +210,7 @@ SrvAgent::raiseEvent(svc_action_t *op, enum service_id service, const char *user event.setProperty("expected-rc", op->expected_rc); }
- if(userdata) { + if (userdata.length()) { event.setProperty("userdata", userdata); }
@@ -356,18 +356,19 @@ SrvAgent::invoke_resources(qmf::AgentSession session, qmf::AgentEvent event, } else if (methodName == "list") { GList *gIter = NULL; GList *agents = NULL; - const char *standard = "ocf"; - const char *provider = "heartbeat"; + std::string standard("ocf"); + std::string provider("heartbeat"); _qtype::Variant::List t_list;
- if(args.count("standard") > 0) { - standard = args["standard"].asString().c_str(); + if (args.count("standard")) { + standard = args["standard"].asString(); } - if(args.count("provider") > 0) { - provider = args["provider"].asString().c_str(); + + if (args.count("provider")) { + provider = args["provider"].asString(); }
- agents = resources_list_agents(standard, provider); + agents = resources_list_agents(standard.c_str(), provider.c_str()); for (gIter = agents; gIter != NULL; gIter = gIter->next) { t_list.push_back((const char *) gIter->data); } @@ -389,9 +390,9 @@ SrvAgent::invoke_resources(qmf::AgentSession session, qmf::AgentEvent event,
int32_t interval = 0; int32_t timeout = 60000; - const char *agent = NULL; - const char *standard = "ocf"; - const char *provider = "heartbeat"; + std::string agent; + std::string standard("ocf"); + std::string provider("heartbeat");
for ( iter=standards.begin() ; iter != standards.end(); iter++ ) { if(args["standard"].asString() == (*iter).asString()) { @@ -406,16 +407,16 @@ SrvAgent::invoke_resources(qmf::AgentSession session, qmf::AgentEvent event, return TRUE; }
- if(args.count("standard") > 0) { - standard = args["standard"].asString().c_str(); + if (args.count("standard")) { + standard = args["standard"].asString(); } - if(args.count("provider") > 0) { - provider = args["provider"].asString().c_str(); + if (args.count("provider")) { + provider = args["provider"].asString(); } - if(args.count("agent") > 0) { - agent = args["agent"].asString().c_str(); + if (args.count("agent")) { + agent = args["agent"].asString(); } else { - agent = args["name"].asString().c_str(); + agent = args["name"].asString(); }
if(args.count("interval") > 0) { @@ -427,7 +428,7 @@ SrvAgent::invoke_resources(qmf::AgentSession session, qmf::AgentEvent event,
op = resources_action_create( args["name"].asString().c_str(), - standard, provider, agent, + standard.c_str(), provider.c_str(), agent.c_str(), args["action"].asString().c_str(), interval, timeout, params);
matahari@lists.fedorahosted.org
-
Russell Bryant