From 4132c9bbbc2f1e5e593430d04fc7b3115ef3466e Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Mon, 4 Feb 2019 12:13:12 +0200 Subject: [PATCH] MXS-2304 Use get_c_str_copy instead of config_copy_string() Also uses get_string() in core-code when appropriate. --- include/maxscale/config.hh | 30 +++++++++--------- server/core/config.cc | 31 +++++++++---------- server/core/config_runtime.cc | 2 +- server/core/monitor.cc | 6 ++-- server/core/service.cc | 8 ++--- server/core/test/test_config.cc | 12 +++---- server/modules/filter/cache/cachefilter.cc | 2 +- .../filter/insertstream/insertstream.cc | 4 +-- server/modules/filter/luafilter/luafilter.cc | 4 +-- server/modules/filter/mqfilter/mqfilter.cc | 8 ++--- .../modules/filter/regexfilter/regexfilter.cc | 4 +-- server/modules/filter/topfilter/topfilter.cc | 8 ++--- server/modules/filter/tpmfilter/tpmfilter.cc | 4 +-- server/modules/routing/binlogrouter/blr.cc | 17 +++++----- 14 files changed, 67 insertions(+), 73 deletions(-) diff --git a/include/maxscale/config.hh b/include/maxscale/config.hh index 933ca4e04..82985f84d 100644 --- a/include/maxscale/config.hh +++ b/include/maxscale/config.hh @@ -240,6 +240,20 @@ public: */ const char* get_c_str(const std::string& key) const; + /** + * @brief Get copy of parameter value if it is defined + * + * If a parameter with the name of @c key is defined in @c params, a copy of the + * value of that parameter is returned. The caller must free the returned string. + * + * @param key Parameter name + * @return Pointer to copy of value or NULL if the parameter was not found + * + * @note The use of this function should be avoided after startup as the function + * will abort the process if memory allocation fails. + */ + char* get_c_str_copy(const std::string& key) const; + /** * Get an integer value. Should be used for both MXS_MODULE_PARAM_INT and MXS_MODULE_PARAM_COUNT * parameter types. @@ -491,22 +505,6 @@ bool config_get_compiled_regexes(const MXS_CONFIG_PARAMETER* params, */ std::vector config_break_list_string(const char* list_string); -/** - * @brief Get copy of parameter value if it is defined - * - * If a parameter with the name of @c key is defined in @c params, a copy of the - * value of that parameter is returned. The caller must free the returned string. - * - * @param params List of configuration parameters - * @param key Parameter name - * - * @return Pointer to copy of value or NULL if the parameter was not found - * - * @note The use of this function should be avoided after startup as the function - * will abort the process if memory allocation fails. - */ -char* config_copy_string(const MXS_CONFIG_PARAMETER* params, const char* key); - /** * @brief Convert string truth value * diff --git a/server/core/config.cc b/server/core/config.cc index d7a38a06b..7de982905 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -1281,7 +1281,7 @@ const MXS_MODULE* get_module(CONFIG_CONTEXT* obj, const char* param_name, const const char* get_missing_module_parameter_name(const CONFIG_CONTEXT* obj) { - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); if (type == CN_SERVICE && !config_get_param(obj->parameters, CN_ROUTER)) { @@ -1309,7 +1309,7 @@ const char* get_missing_module_parameter_name(const CONFIG_CONTEXT* obj) std::pair get_module_details(const CONFIG_CONTEXT* obj) { - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); if (type == CN_SERVICE) { @@ -1394,18 +1394,18 @@ std::unordered_set get_dependencies(const std::vectorparameters, p[i].name); + std::string v = obj->parameters->get_string(p[i].name); rval.insert(name_to_object(objects, obj, v)); } } } } - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); if (type == CN_SERVICE && config_get_value(obj->parameters, CN_FILTERS)) { - for (std::string name : mxs::strtok(config_get_string(obj->parameters, CN_FILTERS), "|")) + for (std::string name : mxs::strtok(obj->parameters->get_string(CN_FILTERS), "|")) { rval.insert(name_to_object(objects, obj, name)); } @@ -1413,7 +1413,7 @@ std::unordered_set get_dependencies(const std::vectorparameters, CN_SERVERS)) { - for (std::string name : mxs::strtok(config_get_string(obj->parameters, CN_SERVERS), ",")) + for (std::string name : mxs::strtok(obj->parameters->get_string(CN_SERVERS), ",")) { rval.insert(name_to_object(objects, obj, name)); } @@ -1641,7 +1641,7 @@ static bool process_config_context(CONFIG_CONTEXT* context) */ for (CONFIG_CONTEXT* obj : objects) { - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); mxb_assert(!type.empty()); if (type == CN_SERVER) @@ -1663,7 +1663,7 @@ static bool process_config_context(CONFIG_CONTEXT* context) */ for (CONFIG_CONTEXT* obj : objects) { - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); mxb_assert(!type.empty()); if (type == CN_SERVICE) @@ -1836,17 +1836,14 @@ std::vector config_get_server_list(const MXS_CONFIG_PARAMETER* params, return server_arr; } -char* config_copy_string(const MXS_CONFIG_PARAMETER* params, const char* key) +char* MXS_CONFIG_PARAMETER::get_c_str_copy(const string& key) const { - const char* value = config_get_value_string(params, key); - + string value = get_string(key); char* rval = NULL; - - if (*value) + if (!value.empty()) { - rval = MXS_STRDUP_A(value); + rval = MXS_STRDUP_A(value.c_str()); } - return rval; } @@ -3041,7 +3038,7 @@ static bool check_config_objects(CONFIG_CONTEXT* context) continue; } - std::string type = config_get_string(obj->parameters, CN_TYPE); + std::string type = obj->parameters->get_string(CN_TYPE); if (!valid_object_type(type)) { @@ -3688,7 +3685,7 @@ int create_new_service(CONFIG_CONTEXT* obj) { int error_count = 0; - for (auto& a : mxs::strtok(config_get_string(obj->parameters, CN_SERVERS), ",")) + for (auto& a : mxs::strtok(obj->parameters->get_string(CN_SERVERS), ",")) { fix_object_name(a); diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index f5bfc5996..b035c59e1 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -709,7 +709,7 @@ bool runtime_alter_service(Service* service, const char* zKey, const char* zValu if (service->router->configureInstance && service->capabilities & RCAP_TYPE_RUNTIME_CONFIG) { // Stash the old value in case the reconfiguration fails. - std::string old_value = config_get_string(service->svc_config_param, key.c_str()); + std::string old_value = service->svc_config_param->get_string(key); service_replace_parameter(service, key.c_str(), value.c_str()); if (!service->router->configureInstance(service->router_instance, service->svc_config_param)) diff --git a/server/core/monitor.cc b/server/core/monitor.cc index fb54d1bf7..87b1df530 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -194,11 +194,11 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params) m_settings.interval = params->get_integer(CN_MONITOR_INTERVAL); m_settings.journal_max_age = params->get_integer(CN_JOURNAL_MAX_AGE); m_settings.script_timeout = params->get_integer(CN_SCRIPT_TIMEOUT); - m_settings.script = config_get_string(params, CN_SCRIPT); + m_settings.script = params->get_string(CN_SCRIPT); m_settings.events = params->get_enum(CN_EVENTS, mxs_monitor_event_enum_values); m_settings.disk_space_check_interval = params->get_integer(CN_DISK_SPACE_CHECK_INTERVAL); - m_settings.conn_settings.username = config_get_string(params, CN_USER); - m_settings.conn_settings.password = config_get_string(params, CN_PASSWORD); + m_settings.conn_settings.username = params->get_string(CN_USER); + m_settings.conn_settings.password = params->get_string(CN_PASSWORD); // The monitor serverlist has already been checked to be valid. Empty value is ok too. auto servers_temp = config_get_server_list(params, CN_SERVERS); diff --git a/server/core/service.cc b/server/core/service.cc index 40e0cfde8..2d91c6125 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -133,7 +133,7 @@ Service* service_alloc(const char* name, const char* router, MXS_CONFIG_PARAMETE static std::string get_version_string(MXS_CONFIG_PARAMETER* params) { - std::string version_string = config_get_string(params, CN_VERSION_STRING); + std::string version_string = params->get_string(CN_VERSION_STRING); if (!version_string.empty() && version_string[0] != '5') { @@ -153,9 +153,9 @@ Service::Service(const std::string& service_name, MXS_CONFIG_PARAMETER* params) : m_name(service_name) , m_router_name(router_name) - , m_user(config_get_string(params, CN_USER)) - , m_password(config_get_string(params, CN_PASSWORD)) - , m_weightby(config_get_string(params, CN_WEIGHTBY)) + , m_user(params->get_string(CN_USER)) + , m_password(params->get_string(CN_PASSWORD)) + , m_weightby(params->get_string(CN_WEIGHTBY)) , m_version_string(get_version_string(params)) , m_rate_limits(config_threadcount()) , m_wkey(mxs_rworker_create_key()) diff --git a/server/core/test/test_config.cc b/server/core/test/test_config.cc index 27d2d51f1..f4fa97408 100644 --- a/server/core/test/test_config.cc +++ b/server/core/test/test_config.cc @@ -158,10 +158,10 @@ int test_add_parameter() TEST(ctx.parameters->get_integer("p1") == -123); TEST(ctx.parameters->get_integer("p2") == 123); TEST(ctx.parameters->get_bool("p3") == true); - TEST(strcmp(config_get_string(ctx.parameters, "p4"), "default") == 0); + TEST(ctx.parameters->get_string("p4") == "default"); TEST(ctx.parameters->get_enum("p5", enum_values) == 1); - TEST(strcmp(config_get_string(ctx.parameters, "p6"), "/tmp") == 0); - TEST(strcmp(config_get_string(ctx.parameters, "p7"), "my-service") == 0); + TEST(ctx.parameters->get_string("p6") == "/tmp"); + TEST(ctx.parameters->get_string("p7") == "my-service"); config_parameter_free(ctx.parameters); ctx.parameters = NULL; @@ -180,11 +180,11 @@ int test_add_parameter() TEST(ctx.parameters->get_integer("p1") == -321); TEST(ctx.parameters->get_integer("p2") == 321); TEST(config_get_param(ctx.parameters, "p3") && ctx.parameters->get_bool("p3") == false); - TEST(strcmp(config_get_string(ctx.parameters, "p4"), "strange") == 0); + TEST(ctx.parameters->get_string("p4") == "strange"); int val = ctx.parameters->get_enum("p5", enum_values); TEST(val == 5); - TEST(strcmp(config_get_string(ctx.parameters, "p6"), "/dev/null") == 0); - TEST(strcmp(config_get_string(ctx.parameters, "p7"), "some-service") == 0); + TEST(ctx.parameters->get_string("p6") == "/dev/null"); + TEST(ctx.parameters->get_string("p7") == "some-service"); config_parameter_free(ctx.parameters); config_parameter_free(svc1.parameters); config_parameter_free(svc2.parameters); diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc index 02d013fda..8f86ee50c 100644 --- a/server/modules/filter/cache/cachefilter.cc +++ b/server/modules/filter/cache/cachefilter.cc @@ -375,7 +375,7 @@ bool CacheFilter::process_params(MXS_CONFIG_PARAMETER* ppParams, CACHE_CONFIG& c error = true; } - config.rules = config_copy_string(ppParams, "rules"); + config.rules = ppParams->get_c_str_copy("rules"); const MXS_CONFIG_PARAMETER* pParam = config_get_param(ppParams, "storage_options"); diff --git a/server/modules/filter/insertstream/insertstream.cc b/server/modules/filter/insertstream/insertstream.cc index 07a4c173e..1df55be5f 100644 --- a/server/modules/filter/insertstream/insertstream.cc +++ b/server/modules/filter/insertstream/insertstream.cc @@ -175,8 +175,8 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params if ((my_instance = static_cast(MXS_CALLOC(1, sizeof(DS_INSTANCE)))) != NULL) { - my_instance->source = config_copy_string(params, "source"); - my_instance->user = config_copy_string(params, "user"); + my_instance->source = params->get_c_str_copy("source"); + my_instance->user = params->get_c_str_copy("user"); } return (MXS_FILTER*) my_instance; diff --git a/server/modules/filter/luafilter/luafilter.cc b/server/modules/filter/luafilter/luafilter.cc index ff7f3d8dd..d955631c7 100644 --- a/server/modules/filter/luafilter/luafilter.cc +++ b/server/modules/filter/luafilter/luafilter.cc @@ -262,8 +262,8 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params return NULL; } - my_instance->global_script = config_copy_string(params, "global_script"); - my_instance->session_script = config_copy_string(params, "session_script"); + my_instance->global_script = params->get_c_str_copy("global_script"); + my_instance->session_script = params->get_c_str_copy("session_script"); my_instance->global_lua_state = nullptr; if (my_instance->global_script) diff --git a/server/modules/filter/mqfilter/mqfilter.cc b/server/modules/filter/mqfilter/mqfilter.cc index 3be08b877..479ba72dd 100644 --- a/server/modules/filter/mqfilter/mqfilter.cc +++ b/server/modules/filter/mqfilter/mqfilter.cc @@ -595,10 +595,10 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params my_instance->exchange = MXS_STRDUP_A(config_get_string(params, "exchange")); my_instance->key = MXS_STRDUP_A(config_get_string(params, "key")); my_instance->exchange_type = MXS_STRDUP_A(config_get_string(params, "exchange_type")); - my_instance->queue = config_copy_string(params, "queue"); - my_instance->ssl_client_cert = config_copy_string(params, "ssl_client_certificate"); - my_instance->ssl_client_key = config_copy_string(params, "ssl_client_key"); - my_instance->ssl_CA_cert = config_copy_string(params, "ssl_CA_cert"); + my_instance->queue = params->get_c_str_copy("queue"); + my_instance->ssl_client_cert = params->get_c_str_copy("ssl_client_certificate"); + my_instance->ssl_client_key = params->get_c_str_copy("ssl_client_key"); + my_instance->ssl_CA_cert = params->get_c_str_copy("ssl_CA_cert"); if (my_instance->trgtype & TRG_SOURCE) { diff --git a/server/modules/filter/regexfilter/regexfilter.cc b/server/modules/filter/regexfilter/regexfilter.cc index c87442bcb..2e7c2c10f 100644 --- a/server/modules/filter/regexfilter/regexfilter.cc +++ b/server/modules/filter/regexfilter/regexfilter.cc @@ -208,8 +208,8 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params { my_instance->match = MXS_STRDUP_A(config_get_string(params, "match")); my_instance->replace = MXS_STRDUP_A(config_get_string(params, "replace")); - my_instance->source = config_copy_string(params, "source"); - my_instance->user = config_copy_string(params, "user"); + my_instance->source = params->get_c_str_copy("source"); + my_instance->user = params->get_c_str_copy("user"); my_instance->log_trace = params->get_bool("log_trace"); const char* logfile = config_get_string(params, "log_file"); diff --git a/server/modules/filter/topfilter/topfilter.cc b/server/modules/filter/topfilter/topfilter.cc index 756694892..2beeeecb6 100644 --- a/server/modules/filter/topfilter/topfilter.cc +++ b/server/modules/filter/topfilter/topfilter.cc @@ -213,10 +213,10 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params { my_instance->sessions = 0; my_instance->topN = params->get_integer("count"); - my_instance->match = config_copy_string(params, "match"); - my_instance->exclude = config_copy_string(params, "exclude"); - my_instance->source = config_copy_string(params, "source"); - my_instance->user = config_copy_string(params, "user"); + my_instance->match = params->get_c_str_copy("match"); + my_instance->exclude = params->get_c_str_copy("exclude"); + my_instance->source = params->get_c_str_copy("source"); + my_instance->user = params->get_c_str_copy("user"); my_instance->filebase = MXS_STRDUP_A(config_get_string(params, "filebase")); int cflags = params->get_enum("options", option_values); diff --git a/server/modules/filter/tpmfilter/tpmfilter.cc b/server/modules/filter/tpmfilter/tpmfilter.cc index 4617e5aec..16eb3b200 100644 --- a/server/modules/filter/tpmfilter/tpmfilter.cc +++ b/server/modules/filter/tpmfilter/tpmfilter.cc @@ -229,8 +229,8 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params my_instance->query_delimiter = MXS_STRDUP_A(config_get_string(params, "query_delimiter")); my_instance->query_delimiter_size = strlen(my_instance->query_delimiter); my_instance->named_pipe = MXS_STRDUP_A(config_get_string(params, "named_pipe")); - my_instance->source = config_copy_string(params, "source"); - my_instance->user = config_copy_string(params, "user"); + my_instance->source = params->get_c_str_copy("source"); + my_instance->user = params->get_c_str_copy("user"); bool error = false; diff --git a/server/modules/routing/binlogrouter/blr.cc b/server/modules/routing/binlogrouter/blr.cc index d25a51ed3..09c093034 100644 --- a/server/modules/routing/binlogrouter/blr.cc +++ b/server/modules/routing/binlogrouter/blr.cc @@ -344,7 +344,7 @@ static MXS_ROUTER* createInstance(SERVICE* service, MXS_CONFIG_PARAMETER* params inst->short_burst = params->get_integer("shortburst"); inst->long_burst = params->get_integer("longburst"); inst->burst_size = params->get_size("burstsize"); - inst->binlogdir = config_copy_string(params, "binlogdir"); + inst->binlogdir = params->get_c_str_copy("binlogdir"); inst->heartbeat = params->get_integer("heartbeat"); inst->retry_interval = params->get_integer("connect_retry"); inst->retry_limit = params->get_integer("master_retry_count"); @@ -352,18 +352,18 @@ static MXS_ROUTER* createInstance(SERVICE* service, MXS_CONFIG_PARAMETER* params inst->mariadb10_compat = params->get_bool("mariadb10-compatibility"); inst->maxwell_compat = params->get_bool("maxwell-compatibility"); inst->trx_safe = params->get_bool("transaction_safety"); - inst->fileroot = config_copy_string(params, "filestem"); + inst->fileroot = params->get_c_str_copy("filestem"); /* Server id */ inst->serverid = params->get_integer("server_id"); /* Identity options */ - inst->set_master_version = config_copy_string(params, "master_version"); - inst->set_master_hostname = config_copy_string(params, "master_hostname"); - inst->set_slave_hostname = config_copy_string(params, "slave_hostname"); + inst->set_master_version = params->get_c_str_copy("master_version"); + inst->set_master_hostname = params->get_c_str_copy("master_hostname"); + inst->set_slave_hostname = params->get_c_str_copy("slave_hostname"); inst->masterid = params->get_integer("master_id"); inst->set_master_server_id = inst->masterid != 0; - inst->master_uuid = config_copy_string(params, "master_uuid"); + inst->master_uuid = params->get_c_str_copy("master_uuid"); inst->set_master_uuid = inst->master_uuid != NULL; /* Slave Heartbeat */ @@ -382,14 +382,13 @@ static MXS_ROUTER* createInstance(SERVICE* service, MXS_CONFIG_PARAMETER* params /* Binlog encryption */ inst->encryption.enabled = params->get_bool("encrypt_binlog"); inst->encryption.encryption_algorithm = params->get_enum("encryption_algorithm", enc_algo_values); - inst->encryption.key_management_filename = config_copy_string(params, - "encryption_key_file"); + inst->encryption.key_management_filename = params->get_c_str_copy("encryption_key_file"); /* Encryption CTX */ inst->encryption_ctx = NULL; /* Set router uuid */ - inst->uuid = config_copy_string(params, "uuid"); + inst->uuid = params->get_c_str_copy("uuid"); /* Set Flat storage of binlog files as default */ inst->storage_type = BLR_BINLOG_STORAGE_FLAT;