From 49f7d173b432408c55b40bc529556c9aeff8fc53 Mon Sep 17 00:00:00 2001 From: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Date: Thu, 31 Aug 2023 14:08:05 -0700 Subject: [PATCH] Remove unnecessary use of sds and mem copy in module.c (#12533) Found that in moduleConfigValidityCheck and isModuleConfigNameRegistered, sds is not required. This also allowed to remove unnecessary memcopy from some of the config registering APIs. --- src/module.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/module.c b/src/module.c index 56500f007..4bd901c02 100644 --- a/src/module.c +++ b/src/module.c @@ -12447,7 +12447,7 @@ sds genModulesInfoString(sds info) { * -------------------------------------------------------------------------- */ /* Check if the configuration name is already registered */ -int isModuleConfigNameRegistered(RedisModule *module, sds name) { +int isModuleConfigNameRegistered(RedisModule *module, const char *name) { listNode *match = listSearchKey(module->module_configs, (void *) name); return match != NULL; } @@ -12637,16 +12637,16 @@ int moduleConfigApplyConfig(list *module_configs, const char **err, const char * * -------------------------------------------------------------------------- */ /* Create a module config object. */ -ModuleConfig *createModuleConfig(sds name, RedisModuleConfigApplyFunc apply_fn, void *privdata, RedisModule *module) { +ModuleConfig *createModuleConfig(const char *name, RedisModuleConfigApplyFunc apply_fn, void *privdata, RedisModule *module) { ModuleConfig *new_config = zmalloc(sizeof(ModuleConfig)); - new_config->name = sdsdup(name); + new_config->name = sdsnew(name); new_config->apply_fn = apply_fn; new_config->privdata = privdata; new_config->module = module; return new_config; } -int moduleConfigValidityCheck(RedisModule *module, sds name, unsigned int flags, configType type) { +int moduleConfigValidityCheck(RedisModule *module, const char *name, unsigned int flags, configType type) { if (!module->onload) { errno = EBUSY; return REDISMODULE_ERR; @@ -12762,13 +12762,10 @@ unsigned int maskModuleEnumConfigFlags(unsigned int flags) { * * EALREADY: The provided configuration name is already used. */ int RM_RegisterStringConfig(RedisModuleCtx *ctx, const char *name, const char *default_val, unsigned int flags, RedisModuleConfigGetStringFunc getfn, RedisModuleConfigSetStringFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata) { RedisModule *module = ctx->module; - sds config_name = sdsnew(name); - if (moduleConfigValidityCheck(module, config_name, flags, NUMERIC_CONFIG)) { - sdsfree(config_name); + if (moduleConfigValidityCheck(module, name, flags, NUMERIC_CONFIG)) { return REDISMODULE_ERR; } - ModuleConfig *new_config = createModuleConfig(config_name, applyfn, privdata, module); - sdsfree(config_name); + ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module); new_config->get_fn.get_string = getfn; new_config->set_fn.set_string = setfn; listAddNodeTail(module->module_configs, new_config); @@ -12782,13 +12779,10 @@ int RM_RegisterStringConfig(RedisModuleCtx *ctx, const char *name, const char *d * RedisModule_RegisterStringConfig for detailed information about configs. */ int RM_RegisterBoolConfig(RedisModuleCtx *ctx, const char *name, int default_val, unsigned int flags, RedisModuleConfigGetBoolFunc getfn, RedisModuleConfigSetBoolFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata) { RedisModule *module = ctx->module; - sds config_name = sdsnew(name); - if (moduleConfigValidityCheck(module, config_name, flags, BOOL_CONFIG)) { - sdsfree(config_name); + if (moduleConfigValidityCheck(module, name, flags, BOOL_CONFIG)) { return REDISMODULE_ERR; } - ModuleConfig *new_config = createModuleConfig(config_name, applyfn, privdata, module); - sdsfree(config_name); + ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module); new_config->get_fn.get_bool = getfn; new_config->set_fn.set_bool = setfn; listAddNodeTail(module->module_configs, new_config); @@ -12828,13 +12822,10 @@ int RM_RegisterBoolConfig(RedisModuleCtx *ctx, const char *name, int default_val * See RedisModule_RegisterStringConfig for detailed general information about configs. */ int RM_RegisterEnumConfig(RedisModuleCtx *ctx, const char *name, int default_val, unsigned int flags, const char **enum_values, const int *int_values, int num_enum_vals, RedisModuleConfigGetEnumFunc getfn, RedisModuleConfigSetEnumFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata) { RedisModule *module = ctx->module; - sds config_name = sdsnew(name); - if (moduleConfigValidityCheck(module, config_name, flags, ENUM_CONFIG)) { - sdsfree(config_name); + if (moduleConfigValidityCheck(module, name, flags, ENUM_CONFIG)) { return REDISMODULE_ERR; } - ModuleConfig *new_config = createModuleConfig(config_name, applyfn, privdata, module); - sdsfree(config_name); + ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module); new_config->get_fn.get_enum = getfn; new_config->set_fn.set_enum = setfn; configEnum *enum_vals = zmalloc((num_enum_vals + 1) * sizeof(configEnum)); @@ -12856,13 +12847,10 @@ int RM_RegisterEnumConfig(RedisModuleCtx *ctx, const char *name, int default_val * RedisModule_RegisterStringConfig for detailed information about configs. */ int RM_RegisterNumericConfig(RedisModuleCtx *ctx, const char *name, long long default_val, unsigned int flags, long long min, long long max, RedisModuleConfigGetNumericFunc getfn, RedisModuleConfigSetNumericFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata) { RedisModule *module = ctx->module; - sds config_name = sdsnew(name); - if (moduleConfigValidityCheck(module, config_name, flags, NUMERIC_CONFIG)) { - sdsfree(config_name); + if (moduleConfigValidityCheck(module, name, flags, NUMERIC_CONFIG)) { return REDISMODULE_ERR; } - ModuleConfig *new_config = createModuleConfig(config_name, applyfn, privdata, module); - sdsfree(config_name); + ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module); new_config->get_fn.get_numeric = getfn; new_config->set_fn.set_numeric = setfn; listAddNodeTail(module->module_configs, new_config);