MXS-2304 Add contains() to test if a parameter exists

Replaces uses of config_get_param() in modules either with contains()
or get_string(). The config_get_param() is moved to internal headers,
as it allows seeing inside a config setting.

include/maxscale/config.hh

@@ -318,6 +318,14 @@ public:
      */
     SERVER* get_server(const std::string& key) const;
 
+    /**
+     * Check if a key exists.
+     *
+     * @param key Parameter name
+     * @return True if key was found
+     */
+    bool contains(const std::string& key) const;
+
     char*                    name;          /**< The name of the parameter */
     char*                    value;         /**< The value of the parameter */
     MXS_CONFIG_PARAMETER*    next;          /**< Next pointer in the linked list */
@@ -396,15 +404,6 @@ struct MXS_CONFIG
  */
 MXS_CONFIG* config_get_global_options();
 
-/**
- * @brief Get a configuration parameter
- *
- * @param params List of parameters
- * @param name Name of parameter to get
- * @return The parameter or NULL if the parameter was not found
- */
-MXS_CONFIG_PARAMETER* config_get_param(MXS_CONFIG_PARAMETER* params, const char* name);
-
 /**
  * @brief Helper function for checking SSL parameters
  *

server/core/config.cc

@@ -760,7 +760,7 @@ static int ini_handler(void* userdata, const char* section, const char* name, co
         cntxt->next = ptr;
     }
 
-    if (config_get_param(ptr->parameters, name))
+    if (ptr->parameters->contains(name))
     {
         /** The values in the persisted configurations are updated versions of
          * the ones in the main configuration file.  */
@@ -1283,27 +1283,18 @@ const char* get_missing_module_parameter_name(const CONFIG_CONTEXT* obj)
 {
     std::string type = obj->parameters->get_string(CN_TYPE);
 
-    if (type == CN_SERVICE && !config_get_param(obj->parameters, CN_ROUTER))
+    if (type == CN_SERVICE && !obj->parameters->contains(CN_ROUTER))
     {
         return CN_ROUTER;
     }
-    else if (type == CN_LISTENER && !config_get_param(obj->parameters, CN_PROTOCOL))
+    else if ((type == CN_LISTENER || type == CN_SERVER) && !obj->parameters->contains(CN_PROTOCOL))
     {
         return CN_PROTOCOL;
     }
-    else if (type == CN_SERVER && !config_get_param(obj->parameters, CN_PROTOCOL))
+    else if ((type == CN_MONITOR || type == CN_FILTER) && !obj->parameters->contains(CN_MODULE))
-    {
-        return CN_PROTOCOL;
-    }
-    else if (type == CN_MONITOR && !config_get_param(obj->parameters, CN_MODULE))
     {
         return CN_MODULE;
     }
-    else if (type == CN_FILTER && !config_get_param(obj->parameters, CN_MODULE))
-    {
-        return CN_MODULE;
-    }
-
     return nullptr;
 }
 
@@ -1709,7 +1700,7 @@ void do_passwd_deprecation(CONFIG_CONTEXT* obj)
 {
     if (auto p = config_get_param(obj->parameters, "passwd"))
     {
-        if (config_get_param(obj->parameters, CN_PASSWORD))
+        if (obj->parameters->contains(CN_PASSWORD))
         {
             MXS_WARNING("Both 'password' and 'passwd' specified. Using value of '%s'.", CN_PASSWORD);
         }
@@ -1814,6 +1805,20 @@ SERVER* MXS_CONFIG_PARAMETER::get_server(const std::string& key) const
     return Server::find_by_unique_name(param_value.c_str());
 }
 
+bool MXS_CONFIG_PARAMETER::contains(const string& key) const
+{
+    auto params = this;
+    while (params)
+    {
+        if (params->name == key)
+        {
+            return true;
+        }
+        params = params->next;
+    }
+    return false;
+}
+
 std::vector<SERVER*> config_get_server_list(const MXS_CONFIG_PARAMETER* params, const char* key,
                                             string* name_error_out)
 {
@@ -1973,7 +1978,7 @@ void config_context_free(CONFIG_CONTEXT* context)
 
 bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
 {
-    mxb_assert(config_get_param(obj->parameters, key) == NULL);
+    mxb_assert(!obj->parameters->contains(key));
     bool rval = false;
     char* my_key = MXS_STRDUP(key);
     char* my_value = MXS_STRDUP(value);
@@ -2877,8 +2882,7 @@ static bool missing_required_parameters(const MXS_MODULE_PARAM* mod_params,
     {
         for (int i = 0; mod_params[i].name; i++)
         {
-            if ((mod_params[i].options & MXS_MODULE_OPT_REQUIRED)
+            if ((mod_params[i].options & MXS_MODULE_OPT_REQUIRED) && !params->contains(mod_params[i].name))
-                && config_get_param(params, mod_params[i].name) == NULL)
             {
                 MXS_ERROR("Mandatory parameter '%s' is not defined for '%s'.",
                           mod_params[i].name,
@@ -3571,8 +3575,7 @@ void config_add_defaults(CONFIG_CONTEXT* ctx, const MXS_MODULE_PARAM* params)
     {
         for (int i = 0; params[i].name; i++)
         {
-            if (params[i].default_value
+            if (params[i].default_value && !ctx->parameters->contains(params[i].name))
-                && config_get_param(ctx->parameters, params[i].name) == NULL)
             {
                 bool rv = config_add_param(ctx, params[i].name, params[i].default_value);
                 MXS_ABORT_IF_FALSE(rv);
@@ -3810,7 +3813,7 @@ int create_new_monitor(CONFIG_CONTEXT* obj, std::set<std::string>& monitored_ser
     MXS_CONFIG_PARAMETER* params = obj->parameters;
     // The config loader has already checked that the server list is mostly ok. However, it cannot
     // check that the server names in the list actually ended up generated.
-    if (config_get_param(params, CN_SERVERS))
+    if (params->contains(CN_SERVERS))
     {
         string name_not_found;
         auto servers = config_get_server_list(params, CN_SERVERS, &name_not_found);
@@ -3991,12 +3994,11 @@ int create_new_filter(CONFIG_CONTEXT* obj)
 bool config_have_required_ssl_params(CONFIG_CONTEXT* obj)
 {
     MXS_CONFIG_PARAMETER* param = obj->parameters;
-
+    return param->contains(CN_SSL)
-    return config_get_param(param, CN_SSL)
+           && param->contains(CN_SSL_KEY)
-           && config_get_param(param, CN_SSL_KEY)
+           && param->contains(CN_SSL_CERT)
-           && config_get_param(param, CN_SSL_CERT)
+           && param->contains(CN_SSL_CA_CERT)
-           && config_get_param(param, CN_SSL_CA_CERT)
+           && (param->get_string(CN_SSL) == CN_REQUIRED);
-           && strcmp(config_get_value_string(param, CN_SSL), CN_REQUIRED) == 0;
 }
 
 bool config_is_ssl_parameter(const char* key)

server/core/internal/config.hh

@@ -236,3 +236,12 @@ void dump_param_list(int file,
  * @return True if the parameter can be modified at runtime
  */
 bool config_can_modify_at_runtime(const char* name);
+
+/**
+ * @brief Get a configuration parameter
+ *
+ * @param params List of parameters
+ * @param name Name of parameter to get
+ * @return The parameter or NULL if the parameter was not found
+ */
+MXS_CONFIG_PARAMETER* config_get_param(MXS_CONFIG_PARAMETER* params, const char* name);

server/core/service.cc

@@ -197,7 +197,7 @@ Service::Service(const std::string& service_name,
     strip_db_esc = params->get_bool(CN_STRIP_DB_ESC);
     session_track_trx_state = params->get_bool(CN_SESSION_TRACK_TRX_STATE);
 
-    if (config_get_param(params, CN_RETAIN_LAST_STATEMENTS))
+    if (params->contains(CN_RETAIN_LAST_STATEMENTS))
     {
         retain_last_statements = params->get_integer(CN_RETAIN_LAST_STATEMENTS);
     }

server/core/test/test_config.cc

@@ -179,7 +179,7 @@ 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(ctx.parameters->contains("p3") && ctx.parameters->get_bool("p3") == false);
     TEST(ctx.parameters->get_string("p4") == "strange");
     int val = ctx.parameters->get_enum("p5", enum_values);
     TEST(val == 5);

server/modules/filter/cache/cachefilter.cc

@@ -377,11 +377,10 @@ bool CacheFilter::process_params(MXS_CONFIG_PARAMETER* ppParams, CACHE_CONFIG& c
 
     config.rules = ppParams->get_c_str_copy("rules");
 
-    const MXS_CONFIG_PARAMETER* pParam = config_get_param(ppParams, "storage_options");
+    string storage_options = ppParams->get_string("storage_options");
-
+    if (!storage_options.empty())
-    if (pParam)
     {
-        config.storage_options = MXS_STRDUP(pParam->value);
+        config.storage_options = MXS_STRDUP(storage_options.c_str());
 
         if (config.storage_options)
         {

server/modules/filter/mqfilter/mqfilter.cc

@@ -618,32 +618,32 @@ static MXS_FILTER* createInstance(const char* name, MXS_CONFIG_PARAMETER* params
             MXS_ABORT_IF_NULL(my_instance->obj_trg);
         }
 
-        MXS_CONFIG_PARAMETER* p = config_get_param(params, "logging_source_user");
+        std::string value = params->get_string("logging_source_user");
 
-        if (p && my_instance->src_trg)
+        if (!value.empty() && my_instance->src_trg)
         {
-            my_instance->src_trg->user = parse_optstr(p->value, ",", &my_instance->src_trg->usize);
+            my_instance->src_trg->user = parse_optstr(value.c_str(), ",", &my_instance->src_trg->usize);
         }
 
-        p = config_get_param(params, "logging_source_host");
+        value = params->get_string("logging_source_host");
 
-        if (p && my_instance->src_trg)
+        if (!value.empty() && my_instance->src_trg)
         {
-            my_instance->src_trg->host = parse_optstr(p->value, ",", &my_instance->src_trg->hsize);
+            my_instance->src_trg->host = parse_optstr(value.c_str(), ",", &my_instance->src_trg->hsize);
         }
 
-        p = config_get_param(params, "logging_schema");
+        value = params->get_string("logging_schema");
 
-        if (p && my_instance->shm_trg)
+        if (!value.empty() && my_instance->shm_trg)
         {
-            my_instance->shm_trg->objects = parse_optstr(p->value, ",", &my_instance->shm_trg->size);
+            my_instance->shm_trg->objects = parse_optstr(value.c_str(), ",", &my_instance->shm_trg->size);
         }
 
-        p = config_get_param(params, "logging_object");
+        value = params->get_string("logging_object");
 
-        if (p && my_instance->obj_trg)
+        if (!value.empty() && my_instance->obj_trg)
         {
-            my_instance->obj_trg->objects = parse_optstr(p->value, ",", &my_instance->obj_trg->size);
+            my_instance->obj_trg->objects = parse_optstr(value.c_str(), ",", &my_instance->obj_trg->size);
         }
 
         my_instance->use_ssl = my_instance->ssl_client_cert

server/modules/routing/avrorouter/avro.cc

@@ -94,11 +94,11 @@ void Avro::read_source_service_options(SERVICE* source)
 Avro* Avro::create(SERVICE* service, SRowEventHandler handler)
 {
     SERVICE* source_service = NULL;
-    MXS_CONFIG_PARAMETER* param = config_get_param(service->svc_config_param, "source");
+    std::string source_name = service->svc_config_param->get_string("source");
 
-    if (param)
+    if (!source_name.empty())
     {
-        SERVICE* source = service_find(param->value);
+        SERVICE* source = service_find(source_name.c_str());
         mxb_assert(source);
 
         if (source)
@@ -119,7 +119,7 @@ Avro* Avro::create(SERVICE* service, SRowEventHandler handler)
         }
         else
         {
-            MXS_ERROR("Service '%s' not found.", param->value);
+            MXS_ERROR("Service '%s' not found.", source_name.c_str());
             return NULL;
         }
     }

server/modules/routing/readwritesplit/readwritesplit.cc

@@ -191,11 +191,11 @@ bool RWSplit::have_enough_servers() const
     return succp;
 }
 
-static void log_router_options_not_supported(SERVICE* service, MXS_CONFIG_PARAMETER* p)
+static void log_router_options_not_supported(SERVICE* service, std::string router_opts)
 {
     std::stringstream ss;
 
-    for (const auto& a : mxs::strtok(p->value, ", \t"))
+    for (const auto& a : mxs::strtok(router_opts, ", \t"))
     {
         ss << a << "\n";
     }
@@ -214,9 +214,10 @@ static void log_router_options_not_supported(SERVICE* service, MXS_CONFIG_PARAME
 
 RWSplit* RWSplit::create(SERVICE* service, MXS_CONFIG_PARAMETER* params)
 {
-    if (MXS_CONFIG_PARAMETER* p = config_get_param(params, CN_ROUTER_OPTIONS))
+
+    if (params->contains(CN_ROUTER_OPTIONS))
     {
-        log_router_options_not_supported(service, p);
+        log_router_options_not_supported(service, params->get_string(CN_ROUTER_OPTIONS));
         return NULL;
     }
 

server/modules/routing/schemarouter/schemarouter.cc

@@ -31,9 +31,10 @@ Config::Config(MXS_CONFIG_PARAMETER* conf)
     ignored_dbs.insert("performance_schema");
 
     // TODO: Don't process this in the router
-    if (MXS_CONFIG_PARAMETER* p = config_get_param(conf, "ignore_databases"))
+    std::string ignored_dbs_str = conf->get_string("ignore_databases");
+    if (!ignored_dbs_str.empty())
     {
-        for (const auto& a : mxs::strtok(p->value, ", \t"))
+        for (const auto& a : mxs::strtok(ignored_dbs_str, ", \t"))
         {
             ignored_dbs.insert(a);
         }

server/modules/routing/schemarouter/schemarouterinstance.cc

@@ -53,7 +53,7 @@ SchemaRouter::~SchemaRouter()
 
 SchemaRouter* SchemaRouter::create(SERVICE* pService, MXS_CONFIG_PARAMETER* params)
 {
-    if ((config_get_param(params, "auth_all_servers")) == NULL)
+    if (!params->contains("auth_all_servers"))
     {
         MXS_NOTICE("Authentication data is fetched from all servers. To disable this "
                    "add 'auth_all_servers=0' to the service.");