MXS-2304 Use configuration class methods instead of separate implementations

Replaces parameter add/set/remove/free.

include/maxscale/config.hh

@@ -345,6 +345,8 @@ public:
      */
     static void remove(MXS_CONFIG_PARAMETER** ppParams, const std::string& key);
 
+    static void free_all(MXS_CONFIG_PARAMETER** ppParams);
+
     char*                    name {nullptr};          /**< The name of the parameter */
     char*                    value {nullptr};         /**< The value of the parameter */
     MXS_CONFIG_PARAMETER*    next {nullptr};          /**< Next pointer in the linked list */

server/core/config.cc

@@ -1908,16 +1908,6 @@ int64_t MXS_CONFIG_PARAMETER::get_integer(const std::string& key) const
     return value.empty() ? 0 : strtoll(value.c_str(), NULL, 10);
 }
 
-MXS_CONFIG_PARAMETER* config_clone_param(const MXS_CONFIG_PARAMETER* param)
-{
-    auto p2 = new MXS_CONFIG_PARAMETER;
-    p2->name = MXS_STRDUP_A(param->name);
-    p2->value = MXS_STRDUP_A(param->value);
-    p2->next = nullptr;
-
-    return p2;
-}
-
 void config_free_one_param(MXS_CONFIG_PARAMETER* p1)
 {
     if (p1)
@@ -1926,20 +1916,6 @@ void config_free_one_param(MXS_CONFIG_PARAMETER* p1)
     }
 }
 
-/**
- * Free a configuration parameter
- * @param p1 Parameter to free
- */
-void config_parameter_free(MXS_CONFIG_PARAMETER* p1)
-{
-    while (p1)
-    {
-        MXS_CONFIG_PARAMETER* tmp = p1;
-        p1 = p1->next;
-        config_free_one_param(tmp);
-    }
-}
-
 void config_context_free(CONFIG_CONTEXT* context)
 {
     CONFIG_CONTEXT* obj;
@@ -1947,7 +1923,7 @@ void config_context_free(CONFIG_CONTEXT* context)
     while (context)
     {
         obj = context->next;
-        config_parameter_free(context->parameters);
+        MXS_CONFIG_PARAMETER::free_all(&context->parameters);
         MXS_FREE(context->object);
         MXS_FREE(context);
         context = obj;
@@ -1957,27 +1933,8 @@ void config_context_free(CONFIG_CONTEXT* context)
 bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
 {
     mxb_assert(!obj->parameters->contains(key));
-    bool rval = false;
+    MXS_CONFIG_PARAMETER::set(&obj->parameters, key, value);
-    char* my_key = MXS_STRDUP(key);
+    return true;
-    char* my_value = MXS_STRDUP(value);
-    auto param = new MXS_CONFIG_PARAMETER;
-
-    if (my_key && my_value)
-    {
-        param->name = my_key;
-        param->value = my_value;
-        param->next = obj->parameters;
-        obj->parameters = param;
-        rval = true;
-    }
-    else
-    {
-        MXS_FREE(my_key);
-        MXS_FREE(my_value);
-        delete param;
-    }
-
-    return rval;
 }
 
 bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
@@ -2085,6 +2042,19 @@ void MXS_CONFIG_PARAMETER::remove(MXS_CONFIG_PARAMETER** ppParams, const string&
     }
 }
 
+void MXS_CONFIG_PARAMETER::free_all(MXS_CONFIG_PARAMETER** ppParams)
+{
+    mxb_assert(ppParams);
+    auto pParam = *ppParams;
+    while (pParam)
+    {
+        auto temp = pParam;
+        pParam = pParam->next;
+        delete temp;
+    }
+    *ppParams = nullptr;
+}
+
 MXS_CONFIG_PARAMETER* MXS_CONFIG_PARAMETER::find(const std::string& key)
 {
     auto param = this;
@@ -2101,40 +2071,13 @@ MXS_CONFIG_PARAMETER* MXS_CONFIG_PARAMETER::find(const std::string& key)
 
 bool config_replace_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
 {
-    if (MXS_CONFIG_PARAMETER* param = config_get_param(obj->parameters, key))
+    MXS_CONFIG_PARAMETER::set(&obj->parameters, key, value);
-    {
-        MXS_FREE(param->value);
-        param->value = MXS_STRDUP(value);
-    }
-    else
-    {
-        config_add_param(obj, key, value);
-    }
-
     return true;
 }
 
 void config_remove_param(CONFIG_CONTEXT* obj, const char* name)
 {
-    if (obj->parameters && strcmp(obj->parameters->name, name) == 0)
+    MXS_CONFIG_PARAMETER::remove(&obj->parameters, name);
-    {
-        MXS_CONFIG_PARAMETER* tmp = obj->parameters;
-        obj->parameters = obj->parameters->next;
-        config_free_one_param(tmp);
-    }
-    else
-    {
-        for (MXS_CONFIG_PARAMETER* p = obj->parameters; p && p->next; p = p->next)
-        {
-            if (strcmp(p->next->name, name) == 0)
-            {
-                MXS_CONFIG_PARAMETER* tmp = p->next;
-                p->next = tmp->next;
-                config_free_one_param(tmp);
-                break;
-            }
-        }
-    }
 }
 
 /**
@@ -4931,7 +4874,7 @@ ParamList::ParamList(std::initializer_list<std::pair<const char*, const char*>>
 
 ParamList::~ParamList()
 {
-    config_parameter_free(m_ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&m_ctx.parameters);
 }
 
 MXS_CONFIG_PARAMETER* ParamList::params()

server/core/config_runtime.cc

@@ -262,7 +262,7 @@ bool runtime_create_server(const char* name,
                                          name);
                 }
 
-                config_parameter_free(ctx.parameters);
+                MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
             }
             else
             {
@@ -1201,7 +1201,7 @@ bool runtime_create_monitor(const char* name, const char* module)
                     config_runtime_error("Could not create monitor '%s' with module '%s'", name, module);
                 }
 
-                config_parameter_free(params);
+                MXS_CONFIG_PARAMETER::free_all(&params);
             }
         }
         else
@@ -1258,7 +1258,7 @@ bool runtime_create_filter(const char* name, const char* module, MXS_CONFIG_PARA
                     config_runtime_error("Could not create filter '%s' with module '%s'", name, module);
                 }
 
-                config_parameter_free(ctx.parameters);
+                MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
             }
             else
             {
@@ -1335,7 +1335,7 @@ static bool runtime_create_service(const char* name, const char* router, MXS_CON
                     config_runtime_error("Could not create service '%s' with module '%s'", name, router);
                 }
 
-                config_parameter_free(ctx.parameters);
+                MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
             }
             else
             {
@@ -2264,7 +2264,7 @@ bool runtime_create_filter_from_json(json_t* json)
 
         rval = runtime_create_filter(name, module, params);
 
-        config_parameter_free(params);
+        MXS_CONFIG_PARAMETER::free_all(&params);
     }
 
     return rval;
@@ -2298,7 +2298,7 @@ Service* runtime_create_service_from_json(json_t* json)
             }
         }
 
-        config_parameter_free(params);
+        MXS_CONFIG_PARAMETER::free_all(&params);
     }
 
     return rval;

server/core/filter.cc

@@ -59,7 +59,7 @@ static struct
  */
 static void filter_free_parameters(FilterDef* filter)
 {
-    config_parameter_free(filter->parameters);
+    MXS_CONFIG_PARAMETER::free_all(&filter->parameters);
 }
 
 /**
@@ -298,24 +298,6 @@ void dListFilters(DCB* dcb)
     }
 }
 
-/**
- * Add a router parameter to a service
- *
- * @param filter        The filter to add the parameter to
- * @param name          The parameter name
- * @param value         The parameter value
- */
-void filter_add_parameter(SFilterDef& filter, const char* name, const char* value)
-{
-    mxb_assert(filter);
-    CONFIG_CONTEXT ctx = {};
-    ctx.object = (char*)"";
-
-    config_add_param(&ctx, name, value);
-    ctx.parameters->next = filter->parameters;
-    filter->parameters = ctx.parameters;
-}
-
 /**
  * Connect the downstream filter chain for a filter.
  *

server/core/internal/config.hh

@@ -75,10 +75,8 @@ void config_set_global_defaults();
 void config_add_defaults(CONFIG_CONTEXT* ctx, const MXS_MODULE_PARAM* params);
 
 char*                 config_clean_string_list(const char* str);
-MXS_CONFIG_PARAMETER* config_clone_param(const MXS_CONFIG_PARAMETER* param);
 bool                  config_load(const char*);
 bool                  config_load_global(const char* filename);
-void                  config_parameter_free(MXS_CONFIG_PARAMETER* p1);
 
 /**
  * @brief Creates an empty configuration context

server/core/internal/filter.hh

@@ -46,7 +46,6 @@ struct FilterDef : public MXS_FILTER_DEF
 
 typedef std::shared_ptr<FilterDef> SFilterDef;
 
-void            filter_add_parameter(SFilterDef& filter_def, const char* name, const char* value);
 SFilterDef      filter_alloc(const char* name, const char* module, MXS_CONFIG_PARAMETER* params);
 MXS_DOWNSTREAM* filter_apply(const SFilterDef& filter_def, MXS_SESSION* session, MXS_DOWNSTREAM* downstream);
 void            filter_free(const SFilterDef& filter);

server/core/internal/monitor.hh

@@ -136,7 +136,6 @@ void monitor_list(DCB*);
 
 bool monitor_add_server(Monitor* mon, SERVER* server);
 void monitor_remove_server(Monitor* mon, SERVER* server);
-void monitor_add_parameters(Monitor* monitor, const MXS_CONFIG_PARAMETER* params);
 
 void monitor_set_journal_max_age(Monitor* mon, time_t value);
 

server/core/monitor.cc

@@ -231,7 +231,7 @@ bool Monitor::configure_base(const MXS_CONFIG_PARAMETER* params)
 
 Monitor::~Monitor()
 {
-    config_parameter_free(parameters);
+    MXS_CONFIG_PARAMETER::free_all(&parameters);
     monitor_server_free_all(m_servers);
     MXS_FREE((const_cast<char*>(m_name)));
 }
@@ -741,34 +741,6 @@ bool Monitor::test_permissions(const string& query)
     return rval;
 }
 
-/**
- * Add parameters to the monitor
- * @param monitor Monitor
- * @param params Config parameters
- */
-void monitor_add_parameters(Monitor* monitor, const MXS_CONFIG_PARAMETER* params)
-{
-    Guard guard(monitor->m_lock);
-    while (params)
-    {
-        MXS_CONFIG_PARAMETER* old = config_get_param(monitor->parameters, params->name);
-
-        if (old)
-        {
-            MXS_FREE(old->value);
-            old->value = MXS_STRDUP_A(params->value);
-        }
-        else
-        {
-            MXS_CONFIG_PARAMETER* clone = config_clone_param(params);
-            clone->next = monitor->parameters;
-            monitor->parameters = clone;
-        }
-
-        params = params->next;
-    }
-}
-
 void monitor_stash_current_status(MXS_MONITORED_SERVER* ptr)
 {
     ptr->mon_prev_status = ptr->server->status;

server/core/service.cc

@@ -246,7 +246,7 @@ Service::~Service()
         MXS_FREE(tmp);
     }
 
-    config_parameter_free(svc_config_param);
+    MXS_CONFIG_PARAMETER::free_all(&svc_config_param);
 }
 
 void service_free(Service* service)
@@ -1122,13 +1122,7 @@ int service_refresh_users(SERVICE* svc)
 
 void service_add_parameters(Service* service, const MXS_CONFIG_PARAMETER* param)
 {
-    while (param)
+    MXS_CONFIG_PARAMETER::set_multiple(&service->svc_config_param, param);
-    {
-        MXS_CONFIG_PARAMETER* new_param = config_clone_param(param);
-        new_param->next = service->svc_config_param;
-        service->svc_config_param = new_param;
-        param = param->next;
-    }
 }
 
 void service_add_parameter(Service* service, const char* key, const char* value)
@@ -1141,52 +1135,12 @@ void service_add_parameter(Service* service, const char* key, const char* value)
 
 void service_remove_parameter(Service* service, const char* key)
 {
-    if (MXS_CONFIG_PARAMETER* params = service->svc_config_param)
+    MXS_CONFIG_PARAMETER::remove(&service->svc_config_param, key);
-    {
-        MXS_CONFIG_PARAMETER* to_free = NULL;
-
-        if (strcasecmp(params->name, key) == 0)
-        {
-            service->svc_config_param = params->next;
-            to_free = params;
-        }
-        else
-        {
-            while (MXS_CONFIG_PARAMETER* p = params->next)
-            {
-                if (strcasecmp(p->name, key) == 0)
-                {
-                    params->next = p->next;
-                    to_free = p;
-                    break;
-                }
-
-                params = p;
-            }
-        }
-
-        if (to_free)
-        {
-            // Set next pointer to null to prevent freeing of other parameters
-            to_free->next = NULL;
-            config_parameter_free(to_free);
-        }
-    }
 }
 
 void service_replace_parameter(Service* service, const char* key, const char* value)
 {
-    for (MXS_CONFIG_PARAMETER* p = service->svc_config_param; p; p = p->next)
+    MXS_CONFIG_PARAMETER::set(&service->svc_config_param, key, value);
-    {
-        if (strcasecmp(p->name, key) == 0)
-        {
-            MXS_FREE(p->value);
-            p->value = MXS_STRDUP_A(value);
-            return;
-        }
-    }
-
-    service_add_parameter(service, key, value);
 }
 
 /**

server/core/test/test_config.cc

@@ -116,7 +116,7 @@ int test_validity()
     TEST(!config_param_is_valid(params, "p8", "d", &ctx));
     TEST(!config_param_is_valid(params, "p8", "a,d", &ctx));
     TEST(!config_param_is_valid(params, "p8", "a,b,c,d", &ctx));
-    config_parameter_free(svc.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&svc.parameters);
     return 0;
 }
 
@@ -163,7 +163,7 @@ int test_add_parameter()
     TEST(ctx.parameters->get_string("p6") == "/tmp");
     TEST(ctx.parameters->get_string("p7") == "my-service");
 
-    config_parameter_free(ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
     ctx.parameters = NULL;
 
     /** Test custom parameters overriding default values */
@@ -185,9 +185,9 @@ int test_add_parameter()
     TEST(val == 5);
     TEST(ctx.parameters->get_string("p6") == "/dev/null");
     TEST(ctx.parameters->get_string("p7") == "some-service");
-    config_parameter_free(ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
-    config_parameter_free(svc1.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&svc1.parameters);
-    config_parameter_free(svc2.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&svc2.parameters);
     return 0;
 }
 
@@ -208,14 +208,14 @@ int test_required_parameters()
     config_add_defaults(&ctx, params);
     TEST(!missing_required_parameters(params, ctx.parameters, "test"));
 
-    config_parameter_free(ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
     ctx.parameters = NULL;
 
     config_add_param(&ctx, "p1", "1");
     config_add_param(&ctx, "p2", "1");
     config_add_param(&ctx, "p3", "1");
     TEST(!missing_required_parameters(params, ctx.parameters, "test"));
-    config_parameter_free(ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
     return 0;
 }
 

server/modules/routing/avrorouter/avro.cc

@@ -62,18 +62,9 @@ using namespace maxscale;
 void Avro::read_source_service_options(SERVICE* source)
 {
     MXS_CONFIG_PARAMETER* params = source->svc_config_param;
-
+    binlogdir = params->get_string("binlogdir");
-    for (MXS_CONFIG_PARAMETER* p = params; p; p = p->next)
+    filestem = params->get_string("filestem");
-    {
+    mxb_assert(!binlogdir.empty() && !filestem.empty());
-        if (strcmp(p->name, "binlogdir") == 0)
-        {
-            binlogdir = p->value;
-        }
-        else if (strcmp(p->name, "filestem") == 0)
-        {
-            filestem = p->value;
-        }
-    }
 
     for (const auto& opt : mxs::strtok(params->get_string("router_options"), ", \t"))
     {

server/modules/routing/binlogrouter/test/testbinlog.cc

@@ -133,7 +133,7 @@ int main(int argc, char** argv)
         return 1;
     }
 
-    config_parameter_free(ctx.parameters);
+    MXS_CONFIG_PARAMETER::free_all(&ctx.parameters);
 
     // Declared in config.cc and needs to be removed if/when blr is refactored
     extern const MXS_MODULE_PARAM config_server_params[];