Commit 488e1947 authored by Oran Agra's avatar Oran Agra Committed by antirez
Browse files

config.c verbose error replies for CONFIG SET, like config file parsing

We noticed that the error replies for the generic mechanism for enums
are very verbose for config file parsing, but not for config set
command.

instead of replicating this code, i did a small refactoring to share
code between CONFIG SET and config file parsing.

and also renamed the enum group functions to be consistent with the
naming of other types.
parent c82ccf06
...@@ -190,8 +190,9 @@ typedef struct typeInterface { ...@@ -190,8 +190,9 @@ typedef struct typeInterface {
void (*init)(typeData data); void (*init)(typeData data);
/* Called on server start, should return 1 on success, 0 on error and should set err */ /* Called on server start, should return 1 on success, 0 on error and should set err */
int (*load)(typeData data, sds *argc, int argv, char **err); int (*load)(typeData data, sds *argc, int argv, char **err);
/* Called on CONFIG SET, returns 1 on success, 0 on error */ /* Called on server startup and CONFIG SET, returns 1 on success, 0 on error
int (*set)(typeData data, sds value, char **err); * and can set a verbose err string, update is true when called from CONFIG SET */
int (*set)(typeData data, sds value, int update, char **err);
/* Called on CONFIG GET, required to add output to the client */ /* Called on CONFIG GET, required to add output to the client */
void (*get)(client *c, typeData data); void (*get)(client *c, typeData data);
/* Called on CONFIG REWRITE, required to rewrite the config state */ /* Called on CONFIG REWRITE, required to rewrite the config state */
...@@ -323,7 +324,11 @@ void loadServerConfigFromString(char *config) { ...@@ -323,7 +324,11 @@ void loadServerConfigFromString(char *config) {
if ((!strcasecmp(argv[0],config->name) || if ((!strcasecmp(argv[0],config->name) ||
(config->alias && !strcasecmp(argv[0],config->alias)))) (config->alias && !strcasecmp(argv[0],config->alias))))
{ {
if (!config->interface.load(config->data, argv, argc, &err)) { if (argc != 2) {
err = "wrong number of arguments";
goto loaderr;
}
if (!config->interface.set(config->data, argv[1], 0, &err)) {
goto loaderr; goto loaderr;
} }
...@@ -599,7 +604,7 @@ void configSetCommand(client *c) { ...@@ -599,7 +604,7 @@ void configSetCommand(client *c) {
if(config->modifiable && (!strcasecmp(c->argv[2]->ptr,config->name) || if(config->modifiable && (!strcasecmp(c->argv[2]->ptr,config->name) ||
(config->alias && !strcasecmp(c->argv[2]->ptr,config->alias)))) (config->alias && !strcasecmp(c->argv[2]->ptr,config->alias))))
{ {
if (!config->interface.set(config->data,o->ptr, &errstr)) { if (!config->interface.set(config->data,o->ptr,1,&errstr)) {
goto badfmt; goto badfmt;
} }
addReply(c,shared.ok); addReply(c,shared.ok);
...@@ -1536,9 +1541,8 @@ static char loadbuf[LOADBUF_SIZE]; ...@@ -1536,9 +1541,8 @@ static char loadbuf[LOADBUF_SIZE];
.alias = (config_alias), \ .alias = (config_alias), \
.modifiable = (is_modifiable), .modifiable = (is_modifiable),
#define embedConfigInterface(initfn, loadfn, setfn, getfn, rewritefn) .interface = { \ #define embedConfigInterface(initfn, setfn, getfn, rewritefn) .interface = { \
.init = (initfn), \ .init = (initfn), \
.load = (loadfn), \
.set = (setfn), \ .set = (setfn), \
.get = (getfn), \ .get = (getfn), \
.rewrite = (rewritefn) \ .rewrite = (rewritefn) \
...@@ -1561,30 +1565,17 @@ static void boolConfigInit(typeData data) { ...@@ -1561,30 +1565,17 @@ static void boolConfigInit(typeData data) {
*data.yesno.config = data.yesno.default_value; *data.yesno.config = data.yesno.default_value;
} }
static int boolConfigLoad(typeData data, sds *argv, int argc, char **err) { static int boolConfigSet(typeData data, sds value, int update, char **err) {
int yn; int yn = yesnotoi(value);
if (argc != 2) { if (yn == -1) {
*err = "wrong number of arguments";
return 0;
}
if ((yn = yesnotoi(argv[1])) == -1) {
*err = "argument must be 'yes' or 'no'"; *err = "argument must be 'yes' or 'no'";
return 0; return 0;
} }
if (data.yesno.is_valid_fn && !data.yesno.is_valid_fn(yn, err))
return 0;
*data.yesno.config = yn;
return 1;
}
static int boolConfigSet(typeData data, sds value, char **err) {
int yn = yesnotoi(value);
if (yn == -1) return 0;
if (data.yesno.is_valid_fn && !data.yesno.is_valid_fn(yn, err)) if (data.yesno.is_valid_fn && !data.yesno.is_valid_fn(yn, err))
return 0; return 0;
int prev = *(data.yesno.config); int prev = *(data.yesno.config);
*(data.yesno.config) = yn; *(data.yesno.config) = yn;
if (data.yesno.update_fn && !data.yesno.update_fn(yn, prev, err)) { if (update && data.yesno.update_fn && !data.yesno.update_fn(yn, prev, err)) {
*(data.yesno.config) = prev; *(data.yesno.config) = prev;
return 0; return 0;
} }
...@@ -1601,7 +1592,7 @@ static void boolConfigRewrite(typeData data, const char *name, struct rewriteCon ...@@ -1601,7 +1592,7 @@ static void boolConfigRewrite(typeData data, const char *name, struct rewriteCon
#define createBoolConfig(name, alias, modifiable, config_addr, default, is_valid, update) { \ #define createBoolConfig(name, alias, modifiable, config_addr, default, is_valid, update) { \
embedCommonConfig(name, alias, modifiable) \ embedCommonConfig(name, alias, modifiable) \
embedConfigInterface(boolConfigInit, boolConfigLoad, boolConfigSet, boolConfigGet, boolConfigRewrite) \ embedConfigInterface(boolConfigInit, boolConfigSet, boolConfigGet, boolConfigRewrite) \
.data.yesno = { \ .data.yesno = { \
.config = &(config_addr), \ .config = &(config_addr), \
.default_value = (default), \ .default_value = (default), \
...@@ -1619,23 +1610,7 @@ static void stringConfigInit(typeData data) { ...@@ -1619,23 +1610,7 @@ static void stringConfigInit(typeData data) {
} }
} }
static int stringConfigLoad(typeData data, sds *argv, int argc, char **err) { static int stringConfigSet(typeData data, sds value, int update, char **err) {
if (argc != 2) {
*err = "wrong number of arguments";
return 0;
}
if (data.string.is_valid_fn && !data.string.is_valid_fn(argv[1], err))
return 0;
zfree(*data.string.config);
if (data.string.convert_empty_to_null) {
*data.string.config = argv[1][0] ? zstrdup(argv[1]) : NULL;
} else {
*data.string.config = zstrdup(argv[1]);
}
return 1;
}
static int stringConfigSet(typeData data, sds value, char **err) {
if (data.string.is_valid_fn && !data.string.is_valid_fn(value, err)) if (data.string.is_valid_fn && !data.string.is_valid_fn(value, err))
return 0; return 0;
char *prev = *data.string.config; char *prev = *data.string.config;
...@@ -1644,7 +1619,7 @@ static int stringConfigSet(typeData data, sds value, char **err) { ...@@ -1644,7 +1619,7 @@ static int stringConfigSet(typeData data, sds value, char **err) {
} else { } else {
*data.string.config = zstrdup(value); *data.string.config = zstrdup(value);
} }
if (data.string.update_fn && !data.string.update_fn(*data.string.config, prev, err)) { if (update && data.string.update_fn && !data.string.update_fn(*data.string.config, prev, err)) {
zfree(*data.string.config); zfree(*data.string.config);
*data.string.config = prev; *data.string.config = prev;
return 0; return 0;
...@@ -1666,7 +1641,7 @@ static void stringConfigRewrite(typeData data, const char *name, struct rewriteC ...@@ -1666,7 +1641,7 @@ static void stringConfigRewrite(typeData data, const char *name, struct rewriteC
#define createStringConfig(name, alias, modifiable, empty_to_null, config_addr, default, is_valid, update) { \ #define createStringConfig(name, alias, modifiable, empty_to_null, config_addr, default, is_valid, update) { \
embedCommonConfig(name, alias, modifiable) \ embedCommonConfig(name, alias, modifiable) \
embedConfigInterface(stringConfigInit, stringConfigLoad, stringConfigSet, stringConfigGet, stringConfigRewrite) \ embedConfigInterface(stringConfigInit, stringConfigSet, stringConfigGet, stringConfigRewrite) \
.data.string = { \ .data.string = { \
.config = &(config_addr), \ .config = &(config_addr), \
.default_value = (default), \ .default_value = (default), \
...@@ -1677,17 +1652,12 @@ static void stringConfigRewrite(typeData data, const char *name, struct rewriteC ...@@ -1677,17 +1652,12 @@ static void stringConfigRewrite(typeData data, const char *name, struct rewriteC
} }
/* Enum configs */ /* Enum configs */
static void configEnumInit(typeData data) { static void enumConfigInit(typeData data) {
*data.enumd.config = data.enumd.default_value; *data.enumd.config = data.enumd.default_value;
} }
static int configEnumLoad(typeData data, sds *argv, int argc, char **err) { static int enumConfigSet(typeData data, sds value, int update, char **err) {
if (argc != 2) { int enumval = configEnumGetValue(data.enumd.enum_value, value);
*err = "wrong number of arguments";
return 0;
}
int enumval = configEnumGetValue(data.enumd.enum_value, argv[1]);
if (enumval == INT_MIN) { if (enumval == INT_MIN) {
sds enumerr = sdsnew("argument must be one of the following: "); sds enumerr = sdsnew("argument must be one of the following: ");
configEnum *enumNode = data.enumd.enum_value; configEnum *enumNode = data.enumd.enum_value;
...@@ -1707,37 +1677,28 @@ static int configEnumLoad(typeData data, sds *argv, int argc, char **err) { ...@@ -1707,37 +1677,28 @@ static int configEnumLoad(typeData data, sds *argv, int argc, char **err) {
*err = loadbuf; *err = loadbuf;
return 0; return 0;
} }
if (data.enumd.is_valid_fn && !data.enumd.is_valid_fn(enumval, err))
return 0;
*(data.enumd.config) = enumval;
return 1;
}
static int configEnumSet(typeData data, sds value, char **err) {
int enumval = configEnumGetValue(data.enumd.enum_value, value);
if (enumval == INT_MIN) return 0;
if (data.enumd.is_valid_fn && !data.enumd.is_valid_fn(enumval, err)) if (data.enumd.is_valid_fn && !data.enumd.is_valid_fn(enumval, err))
return 0; return 0;
int prev = *(data.enumd.config); int prev = *(data.enumd.config);
*(data.enumd.config) = enumval; *(data.enumd.config) = enumval;
if (data.enumd.update_fn && !data.enumd.update_fn(enumval, prev, err)) { if (update && data.enumd.update_fn && !data.enumd.update_fn(enumval, prev, err)) {
*(data.enumd.config) = prev; *(data.enumd.config) = prev;
return 0; return 0;
} }
return 1; return 1;
} }
static void configEnumGet(client *c, typeData data) { static void enumConfigGet(client *c, typeData data) {
addReplyBulkCString(c, configEnumGetNameOrUnknown(data.enumd.enum_value,*data.enumd.config)); addReplyBulkCString(c, configEnumGetNameOrUnknown(data.enumd.enum_value,*data.enumd.config));
} }
static void configEnumRewrite(typeData data, const char *name, struct rewriteConfigState *state) { static void enumConfigRewrite(typeData data, const char *name, struct rewriteConfigState *state) {
rewriteConfigEnumOption(state, name,*(data.enumd.config), data.enumd.enum_value, data.enumd.default_value); rewriteConfigEnumOption(state, name,*(data.enumd.config), data.enumd.enum_value, data.enumd.default_value);
} }
#define createEnumConfig(name, alias, modifiable, enum, config_addr, default, is_valid, update) { \ #define createEnumConfig(name, alias, modifiable, enum, config_addr, default, is_valid, update) { \
embedCommonConfig(name, alias, modifiable) \ embedCommonConfig(name, alias, modifiable) \
embedConfigInterface(configEnumInit, configEnumLoad, configEnumSet, configEnumGet, configEnumRewrite) \ embedConfigInterface(enumConfigInit, enumConfigSet, enumConfigGet, enumConfigRewrite) \
.data.enumd = { \ .data.enumd = { \
.config = &(config_addr), \ .config = &(config_addr), \
.default_value = (default), \ .default_value = (default), \
...@@ -1832,49 +1793,22 @@ static int numericBoundaryCheck(typeData data, long long ll, char **err) { ...@@ -1832,49 +1793,22 @@ static int numericBoundaryCheck(typeData data, long long ll, char **err) {
return 1; return 1;
} }
static int numericConfigLoad(typeData data, sds *argv, int argc, char **err) { static int numericConfigSet(typeData data, sds value, int update, char **err) {
long long ll; long long ll, prev = 0;
if (argc != 2) {
*err = "wrong number of arguments";
return 0;
}
if (data.numeric.is_memory) { if (data.numeric.is_memory) {
int memerr; int memerr;
ll = memtoll(argv[1], &memerr); ll = memtoll(value, &memerr);
if (memerr || ll < 0) { if (memerr || ll < 0) {
*err = "argument must be a memory value"; *err = "argument must be a memory value";
return 0; return 0;
} }
} else { } else {
if (!string2ll(argv[1], sdslen(argv[1]),&ll)) { if (!string2ll(value, sdslen(value),&ll)) {
*err = "argument couldn't be parsed into an integer" ; *err = "argument couldn't be parsed into an integer" ;
return 0; return 0;
} }
} }
if (!numericBoundaryCheck(data, ll, err))
return 0;
if (data.numeric.is_valid_fn && !data.numeric.is_valid_fn(ll, err))
return 0;
SET_NUMERIC_TYPE(ll)
return 1;
}
static int numericConfigSet(typeData data, sds value, char **err) {
long long ll, prev = 0;
if (data.numeric.is_memory) {
int memerr;
ll = memtoll(value, &memerr);
if (memerr || ll < 0) return 0;
} else {
if (!string2ll(value, sdslen(value),&ll)) return 0;
}
if (!numericBoundaryCheck(data, ll, err)) if (!numericBoundaryCheck(data, ll, err))
return 0; return 0;
...@@ -1884,7 +1818,7 @@ static int numericConfigSet(typeData data, sds value, char **err) { ...@@ -1884,7 +1818,7 @@ static int numericConfigSet(typeData data, sds value, char **err) {
GET_NUMERIC_TYPE(prev) GET_NUMERIC_TYPE(prev)
SET_NUMERIC_TYPE(ll) SET_NUMERIC_TYPE(ll)
if (data.numeric.update_fn && !data.numeric.update_fn(ll, prev, err)) { if (update && data.numeric.update_fn && !data.numeric.update_fn(ll, prev, err)) {
SET_NUMERIC_TYPE(prev) SET_NUMERIC_TYPE(prev)
return 0; return 0;
} }
...@@ -1918,7 +1852,7 @@ static void numericConfigRewrite(typeData data, const char *name, struct rewrite ...@@ -1918,7 +1852,7 @@ static void numericConfigRewrite(typeData data, const char *name, struct rewrite
#define embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) { \ #define embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) { \
embedCommonConfig(name, alias, modifiable) \ embedCommonConfig(name, alias, modifiable) \
embedConfigInterface(numericConfigInit, numericConfigLoad, numericConfigSet, numericConfigGet, numericConfigRewrite) \ embedConfigInterface(numericConfigInit, numericConfigSet, numericConfigGet, numericConfigRewrite) \
.data.numeric = { \ .data.numeric = { \
.lower_bound = (lower), \ .lower_bound = (lower), \
.upper_bound = (upper), \ .upper_bound = (upper), \
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment