Return std::string in server_status()

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);
 }

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);
+            printf("\tServer status:            %s\n", statusname.c_str());
-            MXS_FREE(statusname);
         }
     }
     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);
+        string statusname = mxs::server_status(dcb->server);
-        if (statusname)
+        if (!statusname.empty())
         {
-            dcb_printf(pdcb, "\tServer status:            %s\n", statusname);
+            dcb_printf(pdcb, "\tServer status:            %s\n", statusname.c_str());
-            MXS_FREE(statusname);
         }
     }
     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);
+        string statusname = mxs::server_status(dcb->server);
-        if (statusname)
+        if (!statusname.c_str())
         {
-            dcb_printf(pdcb, "\tServer status:            %s\n", statusname);
+            dcb_printf(pdcb, "\tServer status:            %s\n", statusname.c_str());
-            MXS_FREE(statusname);
         }
     }
     char* rolename = dcb_role_name(dcb);

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);
+    string prev = mxs::server_status(&srv);
-    char* next = server_status(ptr->server);
+    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,
+               prev.c_str(),
-               next);
+               next.c_str());
-    MXS_FREE(prev);
-    MXS_FREE(next);
 }
 
 MXS_MONITOR* monitor_server_in_use(const SERVER* server)

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);
+    string stat = mxs::server_status(server);
-    dcb_printf(dcb, "\tStatus:                              %s\n", stat);
+    dcb_printf(dcb, "\tStatus:                              %s\n", stat.c_str());
-    MXS_FREE(stat);
     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);
+                           stat.c_str());
-                MXS_FREE(stat);
             }
         }
 
@@ -656,13 +654,12 @@ void dListServers(DCB* dcb)
 }
 
 /**
- * Convert a set of  server status flags to a string, the returned
+ * Convert a set of  server status flags to a string.
- * string has been malloc'd and must be free'd by the caller
  *
  * @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<ResultSet> 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);
+    string stat = mxs::server_status(server);
-    json_object_set_new(attr, CN_STATE, json_string(stat));
+    json_object_set_new(attr, CN_STATE, json_string(stat.c_str()));
-    MXS_FREE(stat);
 
     json_object_set_new(attr, CN_VERSION_STRING, json_string(server->version_string));
 

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.");
+    mxb_assert_message(status == "Running", "Status of Server should be Running by default.");
-    if (NULL != status)
-    {
-        MXS_FREE(status);
-    }
     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();