From ca9c52944bab9c6a3b138354a797f380e096adf2 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 19 Dec 2018 18:43:55 +0200 Subject: [PATCH] MXS-2220 Use std::string for protocol and authenticator fields --- examples/roundrobinrouter.cpp | 6 ++--- include/maxscale/server.hh | 10 +++++--- server/core/backend.cc | 2 +- server/core/config_runtime.cc | 2 +- server/core/dcb.cc | 5 ++-- server/core/internal/server.hh | 17 ++++++++++++- server/core/modulecmd.cc | 2 +- server/core/server.cc | 25 ++++++------------- server/core/service.cc | 2 +- server/core/test/test_server.cc | 6 ++--- .../modules/routing/hintrouter/hintrouter.cc | 2 +- .../routing/readconnroute/readconnroute.cc | 4 +-- 12 files changed, 44 insertions(+), 39 deletions(-) diff --git a/examples/roundrobinrouter.cpp b/examples/roundrobinrouter.cpp index 516fce0a7..1a4a911b1 100644 --- a/examples/roundrobinrouter.cpp +++ b/examples/roundrobinrouter.cpp @@ -196,9 +196,7 @@ RRRouterSession* RRRouter::create_session(MXS_SESSION* session) if (SERVER_REF_IS_ACTIVE(sref) && (backends.size() < m_max_backends)) { /* Connect to server */ - DCB* conn = dcb_connect(sref->server, - session, - sref->server->protocol); + DCB* conn = dcb_connect(sref->server, session, sref->server->protocol().c_str()); if (conn) { /* Success */ @@ -211,7 +209,7 @@ RRRouterSession* RRRouter::create_session(MXS_SESSION* session) if (m_write_server) { /* Connect to write backend server. This is not essential. */ - write_dcb = dcb_connect(m_write_server, session, m_write_server->protocol); + write_dcb = dcb_connect(m_write_server, session, m_write_server->protocol().c_str()); if (write_dcb) { /* Success */ diff --git a/include/maxscale/server.hh b/include/maxscale/server.hh index c9afc1437..e8153d26b 100644 --- a/include/maxscale/server.hh +++ b/include/maxscale/server.hh @@ -82,9 +82,6 @@ public: }; // Base settings - char* protocol = nullptr; /**< Backend protocol module name */ - char* authenticator = nullptr; /**< Authenticator module name */ - char address[MAX_ADDRESS_LEN] = {'\0'}; /**< Server hostname/IP-address */ int port = -1; /**< Server port */ int extra_port = -1; /**< Alternative monitor port if normal port fails */ @@ -196,6 +193,13 @@ public: */ virtual const char* name() const = 0; + /** + * Get backend protocol module name. + * + * @return Backend protocol module name of the server + */ + virtual std::string protocol() const = 0; + /** * Update the server port. TODO: Move this to internal class once blr is gone. * diff --git a/server/core/backend.cc b/server/core/backend.cc index 4bc444794..222cd219f 100644 --- a/server/core/backend.cc +++ b/server/core/backend.cc @@ -179,7 +179,7 @@ bool Backend::connect(MXS_SESSION* session, SessionCommandList* sescmd) mxb_assert(!in_use()); bool rval = false; - if ((m_dcb = dcb_connect(m_backend->server, session, m_backend->server->protocol))) + if ((m_dcb = dcb_connect(m_backend->server, session, m_backend->server->protocol().c_str()))) { m_closed = false; m_state = IN_USE; diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 36fda56cb..1396fd448 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -455,7 +455,7 @@ bool runtime_alter_server(Server* server, const char* key, const char* value) return false; } - const MXS_MODULE* mod = get_module(server->protocol, MODULE_PROTOCOL); + const MXS_MODULE* mod = get_module(server->protocol().c_str(), MODULE_PROTOCOL); // As servers allow unknown parameters, we must only validate known parameters if (param_is_known(config_server_params, mod->parameters, key) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 310ad37a4..4c73262a2 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -366,9 +366,8 @@ DCB* dcb_connect(SERVER* srv, MXS_SESSION* session, const char* protocol) dcb->remote = MXS_STRDUP_A(session->client_dcb->remote); } - const char* authenticator = server->authenticator ? - server->authenticator : dcb->func.auth_default ? - dcb->func.auth_default() : "NullAuthDeny"; + const char* authenticator = !server->get_authenticator().empty() ? server->get_authenticator().c_str() : + (dcb->func.auth_default ? dcb->func.auth_default() : "NullAuthDeny"); MXS_AUTHENTICATOR* authfuncs = (MXS_AUTHENTICATOR*)load_module(authenticator, MODULE_AUTHENTICATOR); diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index c40e354eb..9063d3ec7 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -31,11 +31,13 @@ std::unique_ptr serverGetList(); class Server : public SERVER { public: - Server(const std::string& name) + Server(const std::string& name, const std::string& protocol = "", const std::string& authenticator = "") : SERVER() , m_name(name) , m_response_time(maxbase::EMAverage {0.04, 0.35, 500}) { + m_settings.protocol = protocol; + m_settings.authenticator = authenticator; } struct ConfigParameter @@ -121,6 +123,16 @@ public: return m_name.c_str(); } + std::string protocol() const override + { + return m_settings.protocol; + } + + std::string get_authenticator() const + { + return m_settings.authenticator; + } + /** * Get a DCB from the persistent connection pool, if possible * @@ -248,6 +260,9 @@ private: { mutable std::mutex lock; /**< Protects array-like settings from concurrent access */ + std::string protocol; /**< Backend protocol module name */ + std::string authenticator; /**< Authenticator module name */ + /** Disk space thresholds. Can be queried from modules at any time so access must be protected * by mutex. */ MxsDiskSpaceThreshold disk_space_limits; diff --git a/server/core/modulecmd.cc b/server/core/modulecmd.cc index 60e0a64b5..dffceb173 100644 --- a/server/core/modulecmd.cc +++ b/server/core/modulecmd.cc @@ -295,7 +295,7 @@ static bool process_argument(const MODULECMD* cmd, if ((arg->value.server = server_find_by_unique_name((char*)value))) { if (MODULECMD_ALLOW_NAME_MISMATCH(type) - || strcmp(cmd->domain, arg->value.server->protocol) == 0) + || (arg->value.server->protocol() == cmd->domain)) { arg->type.type = MODULECMD_ARG_SERVER; rval = true; diff --git a/server/core/server.cc b/server/core/server.cc index c9dfbd810..6dde0a38c 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -199,17 +199,13 @@ Server* Server::server_alloc(const char* name, MXS_CONFIG_PARAMETER* params) return NULL; } - Server* server = new(std::nothrow) Server(name); - char* my_protocol = MXS_STRDUP(protocol); - char* my_authenticator = MXS_STRDUP(authenticator); + Server* server = new(std::nothrow) Server(name, protocol, authenticator); DCB** persistent = (DCB**)MXS_CALLOC(config_threadcount(), sizeof(*persistent)); - if (!server || !my_protocol || !my_authenticator || !persistent) + if (!server || !persistent) { delete server; MXS_FREE(persistent); - MXS_FREE(my_protocol); - MXS_FREE(my_authenticator); SSL_LISTENER_free(ssl); return NULL; } @@ -225,8 +221,6 @@ Server* Server::server_alloc(const char* name, MXS_CONFIG_PARAMETER* params) server->port = config_get_integer(params, CN_PORT); server->extra_port = config_get_integer(params, CN_EXTRA_PORT); - server->protocol = my_protocol; - server->authenticator = my_authenticator; server->m_settings.persistpoolmax = config_get_integer(params, CN_PERSISTPOOLMAX); server->m_settings.persistmaxtime = config_get_integer(params, CN_PERSISTMAXTIME); server->proxy_protocol = config_get_bool(params, CN_PROXY_PROTOCOL); @@ -283,9 +277,6 @@ void server_free(Server* server) } /* Clean up session and free the memory */ - MXS_FREE(server->protocol); - MXS_FREE(server->authenticator); - if (server->persistent) { int nthr = config_threadcount(); @@ -320,7 +311,7 @@ DCB* Server::get_persistent_dcb(const string& user, const string& ip, const stri && !dcb->dcb_errhandle_called && user == dcb->user && ip == dcb->remote - && protocol == dcb->server->protocol) + && protocol == dcb->server->protocol()) { if (NULL == previous) { @@ -413,7 +404,7 @@ void printServer(const SERVER* server) { printf("Server %p\n", server); printf("\tServer: %s\n", server->address); - printf("\tProtocol: %s\n", server->protocol); + printf("\tProtocol: %s\n", server->protocol().c_str()); printf("\tPort: %d\n", server->port); printf("\tTotal connections: %d\n", server->stats.n_connections); printf("\tCurrent connections: %d\n", server->stats.n_current); @@ -514,7 +505,7 @@ void Server::print_to_dcb(DCB* dcb) const dcb_printf(dcb, "\tServer: %s\n", server->address); 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, "\tProtocol: %s\n", server->m_settings.protocol.c_str()); dcb_printf(dcb, "\tPort: %d\n", server->port); dcb_printf(dcb, "\tServer Version: %s\n", server->version_string().c_str()); dcb_printf(dcb, "\tNode Id: %ld\n", server->node_id); @@ -992,7 +983,7 @@ bool Server::create_server_config(const Server* server, const char* filename) dprintf(file, "[%s]\n", server->name()); dprintf(file, "%s=server\n", CN_TYPE); - const MXS_MODULE* mod = get_module(server->protocol, MODULE_PROTOCOL); + const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL); dump_param_list(file, ParamAdaptor(server->m_settings.all_parameters), {CN_TYPE}, @@ -1180,7 +1171,7 @@ json_t* Server::server_json_attributes(const Server* server) /** Store server parameters in attributes */ json_t* params = json_object(); - const MXS_MODULE* mod = get_module(server->protocol, MODULE_PROTOCOL); + const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL); config_add_module_params_json(ParamAdaptor(server->m_settings.all_parameters), {CN_TYPE}, config_server_params, @@ -1390,7 +1381,7 @@ bool Server::is_custom_parameter(const string& name) const return false; } } - auto module_params = get_module(protocol, MODULE_PROTOCOL)->parameters; + auto module_params = get_module(m_settings.protocol.c_str(), MODULE_PROTOCOL)->parameters; for (int i = 0; module_params[i].name; i++) { if (name == module_params[i].name) diff --git a/server/core/service.cc b/server/core/service.cc index 516425908..557fdc7fb 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -919,7 +919,7 @@ void dprintService(DCB* dcb, SERVICE* svc) "\t\t[%s]:%d Protocol: %s Name: %s\n", server->server->address, server->server->port, - server->server->protocol, + server->server->protocol().c_str(), server->server->name()); } server = server->next; diff --git a/server/core/test/test_server.cc b/server/core/test/test_server.cc index 759e1ab97..bd02c85c0 100644 --- a/server/core/test/test_server.cc +++ b/server/core/test/test_server.cc @@ -97,7 +97,7 @@ static int test1() #define TEST(A, B) do {if (!(A)) {printf(B "\n"); return false;}} while (false) -bool test_load_config(const char* input, SERVER* server) +bool test_load_config(const char* input, Server* server) { DUPLICATE_CONTEXT dcontext; @@ -115,9 +115,9 @@ bool test_load_config(const char* input, SERVER* server) TEST(strcmp(obj->object, server->name()) == 0, "Server names differ"); TEST(strcmp(server->address, config_get_param(param, "address")->value) == 0, "Server addresses differ"); - TEST(strcmp(server->protocol, config_get_param(param, "protocol")->value) == 0, + TEST(server->protocol() == config_get_param(param, "protocol")->value, "Server protocols differ"); - TEST(strcmp(server->authenticator, config_get_param(param, "authenticator")->value) == 0, + TEST(server->get_authenticator() == config_get_param(param, "authenticator")->value, "Server authenticators differ"); TEST(server->port == atoi(config_get_param(param, "port")->value), "Server ports differ"); TEST(Server::server_alloc(obj->object, obj->parameters), "Failed to create server from loaded config"); diff --git a/server/modules/routing/hintrouter/hintrouter.cc b/server/modules/routing/hintrouter/hintrouter.cc index 4cd38d6ed..9adbea3cf 100644 --- a/server/modules/routing/hintrouter/hintrouter.cc +++ b/server/modules/routing/hintrouter/hintrouter.cc @@ -198,7 +198,7 @@ Dcb HintRouter::connect_to_backend(MXS_SESSION* session, { Dcb result(NULL); HR_DEBUG("Connecting to %s.", sref->server->name()); - DCB* new_connection = dcb_connect(sref->server, session, sref->server->protocol); + DCB* new_connection = dcb_connect(sref->server, session, sref->server->protocol().c_str()); if (new_connection) { diff --git a/server/modules/routing/readconnroute/readconnroute.cc b/server/modules/routing/readconnroute/readconnroute.cc index 03171cec4..d7d89f49a 100644 --- a/server/modules/routing/readconnroute/readconnroute.cc +++ b/server/modules/routing/readconnroute/readconnroute.cc @@ -412,9 +412,7 @@ static MXS_ROUTER_SESSION* newSession(MXS_ROUTER* instance, MXS_SESSION* session client_rses->backend = candidate; /** Open the backend connection */ - client_rses->backend_dcb = dcb_connect(candidate->server, - session, - candidate->server->protocol); + client_rses->backend_dcb = dcb_connect(candidate->server, session, candidate->server->protocol().c_str()); if (client_rses->backend_dcb == NULL) {