From 86574c16fb8625668336baa5f064672cac983b8e Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 5 Dec 2018 12:43:40 +0200 Subject: [PATCH] Return std::string in server_status() --- include/maxscale/server.hh | 3 +-- server/core/dcb.cc | 21 ++++++++++----------- server/core/monitor.cc | 10 ++++------ server/core/server.cc | 26 +++++++++++--------------- server/core/test/test_server.cc | 18 +++++------------- 5 files changed, 31 insertions(+), 47 deletions(-) diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index eecba3c28..f30290902 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -478,7 +478,6 @@ extern int server_free(SERVER* server); extern SERVER* server_find_by_unique_name(const char* name); extern int server_find_by_unique_names(char** server_names, int size, SERVER*** output); extern SERVER* server_find(const char* servname, unsigned short port); -extern char* server_status(const SERVER*); extern void server_clear_set_status_nolock(SERVER* server, uint64_t bits_to_clear, uint64_t bits_to_set); extern void server_set_status_nolock(SERVER* server, uint64_t bit); extern void server_clear_status_nolock(SERVER* server, uint64_t bit); @@ -512,7 +511,7 @@ double server_response_time_average(const SERVER* server); namespace maxscale { - +std::string server_status(const SERVER*); bool server_set_status(SERVER* server, int bit, std::string* errmsg_out = NULL); bool server_clear_status(SERVER* server, int bit, std::string* errmsg_out = NULL); } diff --git a/server/core/dcb.cc b/server/core/dcb.cc index b38a46cb9..6ca5771fe 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -56,6 +56,7 @@ using maxscale::RoutingWorker; using maxbase::Worker; +using std::string; // #define DCB_LOG_EVENT_HANDLING #if defined (DCB_LOG_EVENT_HANDLING) @@ -1287,10 +1288,10 @@ void printDCB(DCB* dcb) } if (dcb->server) { - if (char* statusname = server_status(dcb->server)) + string statusname = mxs::server_status(dcb->server); + if (!statusname.empty()) { - printf("\tServer status: %s\n", statusname); - MXS_FREE(statusname); + printf("\tServer status: %s\n", statusname.c_str()); } } char* rolename = dcb_role_name(dcb); @@ -1397,11 +1398,10 @@ void dprintOneDCB(DCB* pdcb, DCB* dcb) } if (dcb->server) { - char* statusname = server_status(dcb->server); - if (statusname) + string statusname = mxs::server_status(dcb->server); + if (!statusname.empty()) { - dcb_printf(pdcb, "\tServer status: %s\n", statusname); - MXS_FREE(statusname); + dcb_printf(pdcb, "\tServer status: %s\n", statusname.c_str()); } } char* rolename = dcb_role_name(dcb); @@ -1561,11 +1561,10 @@ void dprintDCB(DCB* pdcb, DCB* dcb) } if (dcb->server) { - char* statusname = server_status(dcb->server); - if (statusname) + string statusname = mxs::server_status(dcb->server); + if (!statusname.c_str()) { - dcb_printf(pdcb, "\tServer status: %s\n", statusname); - MXS_FREE(statusname); + dcb_printf(pdcb, "\tServer status: %s\n", statusname.c_str()); } } char* rolename = dcb_role_name(dcb); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 4e5d6f742..6ef7a1750 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1544,17 +1544,15 @@ static void mon_log_state_change(MXS_MONITORED_SERVER* ptr) { SERVER srv; srv.status = ptr->mon_prev_status; - char* prev = server_status(&srv); - char* next = server_status(ptr->server); + string prev = mxs::server_status(&srv); + string next = mxs::server_status(ptr->server); MXS_NOTICE("Server changed state: %s[%s:%u]: %s. [%s] -> [%s]", ptr->server->name, ptr->server->address, ptr->server->port, mon_get_event_name(ptr), - prev, - next); - MXS_FREE(prev); - MXS_FREE(next); + prev.c_str(), + next.c_str()); } MXS_MONITOR* monitor_server_in_use(const SERVER* server) diff --git a/server/core/server.cc b/server/core/server.cc index 702bd352b..d59456e24 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -500,9 +500,8 @@ void dprintServer(DCB* dcb, const SERVER* srv) dcb_printf(dcb, "Server %p (%s)\n", server, server->name); dcb_printf(dcb, "\tServer: %s\n", server->address); - char* stat = server_status(server); - dcb_printf(dcb, "\tStatus: %s\n", stat); - MXS_FREE(stat); + string stat = mxs::server_status(server); + dcb_printf(dcb, "\tStatus: %s\n", stat.c_str()); dcb_printf(dcb, "\tProtocol: %s\n", server->protocol); dcb_printf(dcb, "\tPort: %d\n", server->port); dcb_printf(dcb, "\tServer Version: %s\n", server->version_string); @@ -639,15 +638,14 @@ void dListServers(DCB* dcb) { if (server->is_active) { - char* stat = server_status(server); + string stat = mxs::server_status(server); dcb_printf(dcb, "%-18s | %-15s | %5d | %11d | %s\n", server->name, server->address, server->port, server->stats.n_current, - stat); - MXS_FREE(stat); + stat.c_str()); } } @@ -656,13 +654,12 @@ void dListServers(DCB* dcb) } /** - * Convert a set of server status flags to a string, the returned - * string has been malloc'd and must be free'd by the caller + * Convert a set of server status flags to a string. * * @param server The server to return the status of * @return A string representation of the status flags */ -char* server_status(const SERVER* server) +string mxs::server_status(const SERVER* server) { mxb_assert(server); uint64_t server_status = server->status; @@ -682,6 +679,7 @@ char* server_status(const SERVER* server) // TODO: The following values should be revisited at some point, but since they are printed by // the REST API they should not be changed suddenly. Strictly speaking, even the combinations // should not change, but this is more dependant on the monitors and have already changed. + // Also, system tests compare to these strings so the output must stay constant for now. const string maintenance = "Maintenance"; const string master = "Master"; const string relay = "Relay Master"; @@ -721,7 +719,7 @@ char* server_status(const SERVER* server) concatenate_if(status_is_running(server_status), running); concatenate_if(status_is_down(server_status), down); - return MXS_STRDUP(result.c_str()); + return result; } /** @@ -977,10 +975,9 @@ std::unique_ptr serverGetList() { if (server_is_active(server)) { - char* stat = server_status(server); + string stat = mxs::server_status(server); set->add_row({server->name, server->address, std::to_string(server->port), std::to_string(server->stats.n_current), stat}); - MXS_FREE(stat); } } @@ -1406,9 +1403,8 @@ static json_t* server_json_attributes(const SERVER* server) json_object_set_new(attr, CN_PARAMETERS, params); /** Store general information about the server state */ - char* stat = server_status(server); - json_object_set_new(attr, CN_STATE, json_string(stat)); - MXS_FREE(stat); + string stat = mxs::server_status(server); + json_object_set_new(attr, CN_STATE, json_string(stat.c_str())); json_object_set_new(attr, CN_VERSION_STRING, json_string(server->version_string)); diff --git a/server/core/test/test_server.cc b/server/core/test/test_server.cc index d9828bfc8..7a3951785 100644 --- a/server/core/test/test_server.cc +++ b/server/core/test/test_server.cc @@ -59,7 +59,8 @@ static int test1() { SERVER* server; int result; - char* status; + std::string status; + using mxs::server_status; /* Server tests */ fprintf(stderr, "testserver : creating server called MyServer"); @@ -79,23 +80,14 @@ static int test1() mxb_assert_message(server == server_find_by_unique_name("uniquename"), "Should find by unique name."); fprintf(stderr, "\t..done\nTesting Status Setting for Server."); status = server_status(server); - mxb_assert_message(0 == strcmp("Running", status), "Status of Server should be Running by default."); - if (NULL != status) - { - MXS_FREE(status); - } + mxb_assert_message(status == "Running", "Status of Server should be Running by default."); server_set_status_nolock(server, SERVER_MASTER); status = server_status(server); - mxb_assert_message(0 == strcmp("Master, Running", status), "Should find correct status."); + mxb_assert_message(status == "Master, Running", "Should find correct status."); server_clear_status_nolock(server, SERVER_MASTER); - MXS_FREE(status); status = server_status(server); - mxb_assert_message(0 == strcmp("Running", status), + mxb_assert_message(status == "Running", "Status of Server should be Running after master status cleared."); - if (NULL != status) - { - MXS_FREE(status); - } fprintf(stderr, "\t..done\nRun Prints for Server and all Servers."); printServer(server); printAllServers();