From dd16a093420f0e1ad0eeadc70fc19d2dd2e86746 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Thu, 17 Jan 2019 15:49:44 +0200 Subject: [PATCH] MXS-2271 Move some initializers out of monitor_create() --- include/maxscale/monitor.hh | 30 +++++++++-------- server/core/config_runtime.cc | 2 +- server/core/internal/monitor.hh | 6 +--- server/core/modulecmd.cc | 2 +- server/core/monitor.cc | 59 ++++++++++++++------------------- 5 files changed, 44 insertions(+), 55 deletions(-) diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index 75dc4a9d9..32c1878dc 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -218,7 +218,7 @@ public: uint64_t pending_status = 0; /**< Status during current monitor loop */ int64_t disk_space_checked = 0; /**< When was the disk space checked the last time */ - MXS_MONITORED_SERVER* next = nullptr; /**< The next server in the list */ + MXS_MONITORED_SERVER* next = nullptr; /**< The next server in the list */ }; namespace std @@ -245,24 +245,28 @@ inline mxb::intrusive_slist_iterator end(MXS_MONITORED_SER class MXS_MONITOR { public: - char* name; /**< Monitor instance name */ - char* module_name; /**< Name of the monitor module */ + MXS_MONITOR(const std::string& name, const std::string& module, MXS_MONITOR_API* api); + + char* name; /**< Monitor instance name */ + const std::string module_name; /**< Name of the monitor module */ MXS_MONITOR_API* api; /**< The monitor api */ MXS_MONITOR_INSTANCE* instance; /**< Instance returned from startMonitor */ MXS_MONITOR* next; /**< Next monitor in the linked list */ mutable std::mutex lock; - bool active; /**< True if monitor is active */ - monitor_state_t state; /**< The state of the monitor. This should ONLY be written to by the - * admin thread. */ + bool active = true; /**< True if monitor exists and has not been "destroyed". */ - int check_maintenance_flag; /**< Set when admin requests a maintenance status change. */ - uint64_t ticks; /**< Number of performed monitoring intervals */ + /** The state of the monitor. This should ONLY be written to by the admin thread. */ + monitor_state_t state = MONITOR_STATE_STOPPED; + /** Set when admin requests a maintenance status change. */ + int check_maintenance_flag = SERVER::MAINTENANCE_FLAG_NOCHECK; + + uint64_t ticks = 0; /**< Number of performed monitoring intervals */ uint8_t journal_hash[SHA_DIGEST_LENGTH]; /**< SHA1 hash of the latest written journal */ - MXS_CONFIG_PARAMETER* parameters; /**< Configuration parameters */ - MXS_MONITORED_SERVER* monitored_servers;/**< List of servers the monitor monitors */ + MXS_CONFIG_PARAMETER* parameters = nullptr; /**< Configuration parameters */ + MXS_MONITORED_SERVER* monitored_servers = nullptr; /**< List of servers the monitor monitors */ char user[MAX_MONITOR_USER_LEN]; /**< Monitor username */ char password[MAX_MONITOR_PASSWORD_LEN]; /**< Monitor password */ @@ -281,9 +285,9 @@ public: const char* script; /**< Launchable script. */ uint64_t events; /**< Enabled monitor events. */ - MxsDiskSpaceThreshold* disk_space_threshold; /**< Disk space thresholds */ - int64_t disk_space_check_interval; /**< How often should a disk space check be made - * at most. */ + MxsDiskSpaceThreshold* disk_space_threshold = NULL; /**< Disk space thresholds */ + int64_t disk_space_check_interval = -1; /**< How often should a disk space check be made + * at most. */ }; /** diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 3bb8b0341..625d94eaa 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -561,7 +561,7 @@ bool validate_param(const MXS_MODULE_PARAM* basic, bool do_alter_monitor(MXS_MONITOR* monitor, const char* key, const char* value) { mxb_assert(monitor->state == MONITOR_STATE_STOPPED); - const MXS_MODULE* mod = get_module(monitor->module_name, MODULE_MONITOR); + const MXS_MODULE* mod = get_module(monitor->module_name.c_str(), MODULE_MONITOR); if (!validate_param(config_monitor_params, mod->parameters, key, value)) { diff --git a/server/core/internal/monitor.hh b/server/core/internal/monitor.hh index 607e84ea0..e042a4925 100644 --- a/server/core/internal/monitor.hh +++ b/server/core/internal/monitor.hh @@ -79,9 +79,7 @@ static const MXS_ENUM_VALUE mxs_monitor_event_enum_values[] = std::unique_ptr monitor_get_list(); -MXS_BEGIN_DECLS - -MXS_MONITOR* monitor_create(const char*, const char*, MXS_CONFIG_PARAMETER* params); +MXS_MONITOR* monitor_create(const std::string& name, const std::string& module, MXS_CONFIG_PARAMETER* params); void monitor_destroy(MXS_MONITOR*); void monitor_start(MXS_MONITOR*, const MXS_CONFIG_PARAMETER*); @@ -169,5 +167,3 @@ int monitor_launch_script(MXS_MONITOR* mon, MXS_MONITORED_SERVER* ptr, const cha * @return Return value of the executed script or -1 on error. */ int monitor_launch_command(MXS_MONITOR* mon, MXS_MONITORED_SERVER* ptr, EXTERNCMD* cmd); - -MXS_END_DECLS diff --git a/server/core/modulecmd.cc b/server/core/modulecmd.cc index f0bc3db62..db71b1a82 100644 --- a/server/core/modulecmd.cc +++ b/server/core/modulecmd.cc @@ -329,7 +329,7 @@ static bool process_argument(const MODULECMD* cmd, case MODULECMD_ARG_MONITOR: if ((arg->value.monitor = monitor_find((char*)value))) { - const char* eff_name = mxs_module_get_effective_name(arg->value.monitor->module_name); + const char* eff_name = mxs_module_get_effective_name(arg->value.monitor->module_name.c_str()); if (MODULECMD_ALLOW_NAME_MISMATCH(type) || strcasecmp(cmd->domain, eff_name) == 0) { arg->type.type = MODULECMD_ARG_MONITOR; diff --git a/server/core/monitor.cc b/server/core/monitor.cc index cfd204f03..a03451765 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -100,38 +100,26 @@ static uint64_t all_server_bits = SERVER_RUNNING | SERVER_MAINT | SERVER_MASTER * Create a new monitor, load the associated module for the monitor * and start execution on the monitor. * - * @param name The name of the monitor module to load - * @param module The module to load - * @return The newly created monitor + * @param name The configuration name of the monitor + * @param module The module name to load + * @return The newly created monitor, or NULL on error */ -MXS_MONITOR* monitor_create(const char* name, const char* module, MXS_CONFIG_PARAMETER* params) +MXS_MONITOR* monitor_create(const string& name, const string& module, MXS_CONFIG_PARAMETER* params) { - MXS_MONITOR_API* api = (MXS_MONITOR_API*)load_module(module, MODULE_MONITOR); - - if (api == NULL) + MXS_MONITOR_API* api = (MXS_MONITOR_API*)load_module(module.c_str(), MODULE_MONITOR); + if (!api) { - MXS_ERROR("Unable to load monitor module '%s'.", name); + MXS_ERROR("Unable to load library file for monitor '%s'.", name.c_str()); return NULL; } - char* my_name = MXS_STRDUP(name); - char* my_module = MXS_STRDUP(module); - MXS_MONITOR* mon = new (std::nothrow) MXS_MONITOR(); - - if (!mon || !my_module || !my_name) + MXS_MONITOR* mon = new(std::nothrow) MXS_MONITOR(name, module, api); + if (!mon) { - delete mon; - MXS_FREE(my_name); - MXS_FREE(my_module); return NULL; } mon->api = api; - mon->active = true; - mon->state = MONITOR_STATE_STOPPED; - mon->name = my_name; - mon->module_name = my_module; - mon->monitored_servers = NULL; mon->read_timeout = config_get_integer(params, CN_BACKEND_READ_TIMEOUT); mon->write_timeout = config_get_integer(params, CN_BACKEND_WRITE_TIMEOUT); mon->connect_timeout = config_get_integer(params, CN_BACKEND_CONNECT_TIMEOUT); @@ -141,11 +129,6 @@ MXS_MONITOR* monitor_create(const char* name, const char* module, MXS_CONFIG_PAR mon->script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT); mon->script = config_get_string(params, CN_SCRIPT); mon->events = config_get_enum(params, CN_EVENTS, mxs_monitor_event_enum_values); - mon->check_maintenance_flag = SERVER::MAINTENANCE_FLAG_NOCHECK; - mon->ticks = 0; - mon->parameters = NULL; - memset(mon->journal_hash, 0, sizeof(mon->journal_hash)); - mon->disk_space_threshold = NULL; mon->disk_space_check_interval = config_get_integer(params, CN_DISK_SPACE_CHECK_INTERVAL); for (auto& s : mxs::strtok(config_get_string(params, CN_SERVERS), ",")) @@ -170,13 +153,13 @@ MXS_MONITOR* monitor_create(const char* name, const char* module, MXS_CONFIG_PAR if (!error) { // Store module, used when the monitor is serialized - monitor_set_parameter(mon, CN_MODULE, module); + monitor_set_parameter(mon, CN_MODULE, module.c_str()); monitor_add_parameters(mon, params); if ((mon->instance = mon->api->createInstance(mon)) == NULL) { MXS_ERROR("Unable to create monitor instance for '%s', using module '%s'.", - name, module); + name.c_str(), module.c_str()); error = true; } } @@ -191,12 +174,19 @@ MXS_MONITOR* monitor_create(const char* name, const char* module, MXS_CONFIG_PAR { delete mon; mon = NULL; - MXS_FREE(my_module); - MXS_FREE(my_name); } return mon; } +MXS_MONITOR::MXS_MONITOR(const string& name, const string& module, MXS_MONITOR_API* api) + : module_name(module) + , api(api) +{ + // The strdup is required until name is an std::string. + this->name = MXS_STRDUP_A(name.c_str()); + memset(journal_hash, 0, sizeof(journal_hash)); +} + /** * Free a monitor, first stop the monitor and then remove the monitor from * the chain of monitors and free the memory. @@ -233,7 +223,6 @@ void monitor_destroy(MXS_MONITOR* mon) config_parameter_free(mon->parameters); monitor_server_free_all(mon->monitored_servers); MXS_FREE(mon->name); - MXS_FREE(mon->module_name); delete mon; } @@ -631,7 +620,7 @@ MXS_MONITOR* monitor_repurpose_destroyed(const char* name, const char* module) for (MXS_MONITOR* ptr = allMonitors; ptr; ptr = ptr->next) { - if (strcmp(ptr->name, name) == 0 && strcmp(ptr->module_name, module) == 0) + if (strcmp(ptr->name, name) == 0 && (ptr->module_name == module)) { mxb_assert(!ptr->active); ptr->active = true; @@ -1571,7 +1560,7 @@ static bool create_monitor_config(const MXS_MONITOR* monitor, const char* filena dprintf(file, "\n"); } - const MXS_MODULE* mod = get_module(monitor->module_name, NULL); + const MXS_MODULE* mod = get_module(monitor->module_name.c_str(), NULL); mxb_assert(mod); dump_param_list(file, @@ -1746,7 +1735,7 @@ static const char* monitor_state_to_string(monitor_state_t state) json_t* monitor_parameters_to_json(const MXS_MONITOR* monitor) { json_t* rval = json_object(); - const MXS_MODULE* mod = get_module(monitor->module_name, MODULE_MONITOR); + const MXS_MODULE* mod = get_module(monitor->module_name.c_str(), MODULE_MONITOR); config_add_module_params_json(monitor->parameters, {CN_TYPE, CN_MODULE, CN_SERVERS}, config_monitor_params, @@ -1766,7 +1755,7 @@ json_t* monitor_json_data(const MXS_MONITOR* monitor, const char* host) json_object_set_new(rval, CN_ID, json_string(monitor->name)); json_object_set_new(rval, CN_TYPE, json_string(CN_MONITORS)); - json_object_set_new(attr, CN_MODULE, json_string(monitor->module_name)); + json_object_set_new(attr, CN_MODULE, json_string(monitor->module_name.c_str())); json_object_set_new(attr, CN_STATE, json_string(monitor_state_to_string(monitor->state))); json_object_set_new(attr, CN_TICKS, json_integer(monitor->ticks));