[2946] in BarnOwl Developers
[PATCH 1/2] Make owl_variable's get_tostring_fn return a newly allocated string
daemon@ATHENA.MIT.EDU (David Benjamin)
Mon Feb 28 19:17:54 2011
From: David Benjamin <davidben@MIT.EDU>
To: barnowl-dev@mit.edu
Date: Mon, 28 Feb 2011 19:17:24 -0500
The mess with buffer lengths and whatnot is obnoxious. Note that this
does change semantics slightly: instead of (optionally) filling a buffer
and giving a return code for good vs no-variable/invalid, we now return
NULL for missing variables and always return a string if the variable
exists. This also appears to be more accurate from the perspective of
the calls anyway.
---
commands.c | 16 +++------
functions.c | 9 +++--
owl.h | 7 ++--
tester.c | 12 ++++---
variable.c | 102 ++++++++++++++++++++++++++--------------------------------
5 files changed, 68 insertions(+), 78 deletions(-)
diff --git a/commands.c b/commands.c
index 160e05c..32ac215 100644
--- a/commands.c
+++ b/commands.c
@@ -1613,7 +1613,7 @@ char *owl_command_unset(int argc, const char *const *argv, const char *buff)
char *owl_command_print(int argc, const char *const *argv, const char *buff)
{
const char *var;
- char valbuff[1024];
+ char *value;
if (argc==1) {
owl_function_printallvars();
@@ -1625,9 +1625,10 @@ char *owl_command_print(int argc, const char *const *argv, const char *buff)
var=argv[1];
- if (0 == owl_variable_get_tostring(owl_global_get_vardict(&g),
- var, valbuff, 1024)) {
- owl_function_makemsg("%s = '%s'", var, valbuff);
+ value = owl_variable_get_tostring(owl_global_get_vardict(&g), var);
+ if (value) {
+ owl_function_makemsg("%s = '%s'", var, value);
+ g_free(value);
} else {
owl_function_makemsg("Unknown variable '%s'.", var);
}
@@ -2506,16 +2507,11 @@ char *owl_command_getview(int argc, const char *const *argv, const char *buff)
char *owl_command_getvar(int argc, const char *const *argv, const char *buff)
{
- char tmpbuff[1024];
if (argc != 2) {
owl_function_makemsg("Wrong number of arguments for %s", argv[0]);
return NULL;
}
- if (owl_variable_get_tostring(owl_global_get_vardict(&g),
- argv[1], tmpbuff, 1024)) {
- return NULL;
- }
- return g_strdup(tmpbuff);
+ return owl_variable_get_tostring(owl_global_get_vardict(&g), argv[1]);
}
char *owl_command_getfilter(int argc, const char *const *argv, const char *buf)
diff --git a/functions.c b/functions.c
index ddb3eac..64df043 100644
--- a/functions.c
+++ b/functions.c
@@ -1615,7 +1615,7 @@ void owl_function_getsubs(void)
void owl_function_printallvars(void)
{
const char *name;
- char var[LINE];
+ char *var;
owl_list varnames;
int i, numvarnames;
GString *str = g_string_new("");
@@ -1628,8 +1628,11 @@ void owl_function_printallvars(void)
name = owl_list_get_element(&varnames, i);
if (name && name[0]!='_') {
g_string_append_printf(str, "\n%-20s = ", name);
- owl_variable_get_tostring(owl_global_get_vardict(&g), name, var, LINE);
- g_string_append(str, var);
+ var = owl_variable_get_tostring(owl_global_get_vardict(&g), name);
+ if (var) {
+ g_string_append(str, var);
+ g_free(var);
+ }
}
}
g_string_append(str, "\n");
diff --git a/owl.h b/owl.h
index 89405fb..96640c6 100644
--- a/owl.h
+++ b/owl.h
@@ -260,10 +260,9 @@ typedef struct _owl_variable {
/* returns a reference to the current value.
* WARNING: this approach is hard to make
* thread-safe... */
- int (*get_tostring_fn)(const struct _owl_variable *v,
- char *buf, int bufsize, const void *val);
- /* converts val to a string
- * and puts into buf */
+ char *(*get_tostring_fn)(const struct _owl_variable *v, const void *val);
+ /* converts val to a string;
+ * caller must free the result */
void (*delete_fn)(struct _owl_variable *v);
/* frees val as needed */
} owl_variable;
diff --git a/tester.c b/tester.c
index c9624b1..72d98e5 100644
--- a/tester.c
+++ b/tester.c
@@ -268,7 +268,7 @@ int owl_dict_regtest(void) {
int owl_variable_regtest(void) {
owl_vardict vd;
int numfailed=0;
- char buf[1024];
+ char *value;
const void *v;
printf("# BEGIN testing owl_variable\n");
@@ -276,8 +276,9 @@ int owl_variable_regtest(void) {
FAIL_UNLESS("get bool", 0==owl_variable_get_bool(&vd,"rxping"));
FAIL_UNLESS("get bool (no such)", -1==owl_variable_get_bool(&vd,"mumble"));
- FAIL_UNLESS("get bool as string 1", 0==owl_variable_get_tostring(&vd,"rxping", buf, 1024));
- FAIL_UNLESS("get bool as string 2", 0==strcmp(buf,"off"));
+ FAIL_UNLESS("get bool as string",
+ !strcmp((value = owl_variable_get_tostring(&vd,"rxping")), "off"));
+ g_free(value);
FAIL_UNLESS("set bool 1", 0==owl_variable_set_bool_on(&vd,"rxping"));
FAIL_UNLESS("get bool 2", 1==owl_variable_get_bool(&vd,"rxping"));
FAIL_UNLESS("set bool 3", 0==owl_variable_set_fromstring(&vd,"rxping","off",0,0));
@@ -292,8 +293,9 @@ int owl_variable_regtest(void) {
FAIL_UNLESS("get int", 8==owl_variable_get_int(&vd,"typewinsize"));
FAIL_UNLESS("get int (no such)", -1==owl_variable_get_int(&vd,"mmble"));
- FAIL_UNLESS("get int as string 1", 0==owl_variable_get_tostring(&vd,"typewinsize", buf, 1024));
- FAIL_UNLESS("get int as string 2", 0==strcmp(buf,"8"));
+ FAIL_UNLESS("get int as string",
+ !strcmp((value = owl_variable_get_tostring(&vd,"typewinsize")), "8"));
+ g_free(value);
FAIL_UNLESS("set int 1", 0==owl_variable_set_int(&vd,"typewinsize",12));
FAIL_UNLESS("get int 2", 12==owl_variable_get_int(&vd,"typewinsize"));
FAIL_UNLESS("set int 1b", -1==owl_variable_set_int(&vd,"typewinsize",-3));
diff --git a/variable.c b/variable.c
index 68307ea..d7c7874 100644
--- a/variable.c
+++ b/variable.c
@@ -672,7 +672,7 @@ const char *owl_variable_get_validsettings(const owl_variable *v) {
/* returns 0 on success, prints a status msg if msg is true */
int owl_variable_set_fromstring(owl_vardict *d, const char *name, const char *value, int msg, int requirebool) {
owl_variable *v;
- char buff2[1024];
+ char *tostring;
if (!name) return(-1);
v = owl_dict_find_element(d, name);
if (v == NULL) {
@@ -693,8 +693,9 @@ int owl_variable_set_fromstring(owl_vardict *d, const char *name, const char *va
return -1;
}
if (msg && v->get_tostring_fn) {
- v->get_tostring_fn(v, buff2, 1024, v->val);
- owl_function_makemsg("%s = '%s'", name, buff2);
+ tostring = v->get_tostring_fn(v, v->val);
+ owl_function_makemsg("%s = '%s'", name, tostring);
+ g_free(tostring);
}
return 0;
}
@@ -725,23 +726,23 @@ int owl_variable_set_bool_off(owl_vardict *d, const char *name) {
return owl_variable_set_int(d,name,0);
}
-int owl_variable_get_tostring(const owl_vardict *d, const char *name, char *buf, int bufsize) {
+char *owl_variable_get_tostring(const owl_vardict *d, const char *name) {
owl_variable *v;
- if (!name) return(-1);
+ if (!name) return NULL;
v = owl_dict_find_element(d, name);
- if (v == NULL || !v->get_tostring_fn) return(-1);
- return v->get_tostring_fn(v, buf, bufsize, v->val);
+ if (v == NULL || !v->get_tostring_fn) return NULL;
+ return v->get_tostring_fn(v, v->val);
}
-int owl_variable_get_default_tostring(const owl_vardict *d, const char *name, char *buf, int bufsize) {
+char *owl_variable_get_default_tostring(const owl_vardict *d, const char *name) {
owl_variable *v;
- if (!name) return(-1);
+ if (!name) return NULL;
v = owl_dict_find_element(d, name);
- if (v == NULL || !v->get_tostring_fn) return(-1);
+ if (v == NULL || !v->get_tostring_fn) return NULL;
if (v->type == OWL_VARIABLE_INT || v->type == OWL_VARIABLE_BOOL) {
- return v->get_tostring_fn(v, buf, bufsize, &(v->ival_default));
+ return v->get_tostring_fn(v, &(v->ival_default));
} else {
- return v->get_tostring_fn(v, buf, bufsize, v->pval_default);
+ return v->get_tostring_fn(v, v->pval_default);
}
}
@@ -785,32 +786,28 @@ int owl_variable_get_bool(const owl_vardict *d, const char *name) {
}
void owl_variable_describe(const owl_vardict *d, const char *name, owl_fmtext *fm) {
- char defaultbuf[50];
- char buf[1024];
- int buflen = 1023;
+ char *default_buf;
owl_variable *v;
if (!name
|| (v = owl_dict_find_element(d, name)) == NULL
|| !v->get_fn) {
- snprintf(buf, buflen, " No such variable '%s'\n", name);
- owl_fmtext_append_normal(fm, buf);
+ owl_fmtext_appendf_normal(fm, " No such variable '%s'\n", name);
return;
}
if (v->type == OWL_VARIABLE_INT || v->type == OWL_VARIABLE_BOOL) {
- v->get_tostring_fn(v, defaultbuf, 50, &(v->ival_default));
+ default_buf = v->get_tostring_fn(v, &(v->ival_default));
} else {
- v->get_tostring_fn(v, defaultbuf, 50, v->pval_default);
+ default_buf = v->get_tostring_fn(v, v->pval_default);
}
- snprintf(buf, buflen, OWL_TABSTR "%-20s - %s (default: '%s')\n",
- v->name,
- owl_variable_get_summary(v), defaultbuf);
- owl_fmtext_append_normal(fm, buf);
+ owl_fmtext_appendf_normal(fm, OWL_TABSTR "%-20s - %s (default: '%s')\n",
+ v->name,
+ owl_variable_get_summary(v), default_buf);
+ g_free(default_buf);
}
void owl_variable_get_help(const owl_vardict *d, const char *name, owl_fmtext *fm) {
- char buff[1024];
- int bufflen = 1023;
+ char *tostring;
owl_variable *v;
if (!name
@@ -828,18 +825,19 @@ void owl_variable_get_help(const owl_vardict *d, const char *name, owl_fmtext *f
owl_fmtext_append_normal(fm, "\n\n");
owl_fmtext_append_normal(fm, "Current: ");
- owl_variable_get_tostring(d, name, buff, bufflen);
- owl_fmtext_append_normal(fm, buff);
+ tostring = owl_variable_get_tostring(d, name);
+ owl_fmtext_append_normal(fm, tostring);
+ g_free(tostring);
owl_fmtext_append_normal(fm, "\n\n");
if (v->type == OWL_VARIABLE_INT || v->type == OWL_VARIABLE_BOOL) {
- v->get_tostring_fn(v, buff, bufflen, &(v->ival_default));
+ tostring = v->get_tostring_fn(v, &(v->ival_default));
} else {
- v->get_tostring_fn(v, buff, bufflen, v->pval_default);
+ tostring = v->get_tostring_fn(v, v->pval_default);
}
owl_fmtext_append_normal(fm, "Default: ");
- owl_fmtext_append_normal(fm, buff);
+ owl_fmtext_append_normal(fm, tostring);
owl_fmtext_append_normal(fm, "\n\n");
owl_fmtext_append_normal(fm, "Valid Settings: ");
@@ -851,6 +849,7 @@ void owl_variable_get_help(const owl_vardict *d, const char *name, owl_fmtext *f
owl_fmtext_append_normal(fm, owl_variable_get_description(v));
owl_fmtext_append_normal(fm, "\n\n");
}
+ g_free(tostring);
}
@@ -895,19 +894,15 @@ int owl_variable_bool_set_fromstring_default(owl_variable *v, const char *newval
return (v->set_fn(v, &i));
}
-int owl_variable_bool_get_tostring_default(const owl_variable *v, char* buf, int bufsize, const void *val) {
+char *owl_variable_bool_get_tostring_default(const owl_variable *v, const void *val) {
if (val == NULL) {
- snprintf(buf, bufsize, "<null>");
- return -1;
+ return g_strdup("<null>");
} else if (*(const int*)val == 0) {
- snprintf(buf, bufsize, "off");
- return 0;
+ return g_strdup("off");
} else if (*(const int*)val == 1) {
- snprintf(buf, bufsize, "on");
- return 0;
+ return g_strdup("on");
} else {
- snprintf(buf, bufsize, "<invalid>");
- return -1;
+ return g_strdup("<invalid>");
}
}
@@ -934,13 +929,11 @@ int owl_variable_int_set_fromstring_default(owl_variable *v, const char *newval)
return (v->set_fn(v, &i));
}
-int owl_variable_int_get_tostring_default(const owl_variable *v, char* buf, int bufsize, const void *val) {
+char *owl_variable_int_get_tostring_default(const owl_variable *v, const void *val) {
if (val == NULL) {
- snprintf(buf, bufsize, "<null>");
- return -1;
+ return g_strdup("<null>");
} else {
- snprintf(buf, bufsize, "%d", *(const int*)val);
- return 0;
+ return g_strdup_printf("%d", *(const int*)val);
}
}
@@ -975,25 +968,24 @@ int owl_variable_enum_set_fromstring(owl_variable *v, const char *newval) {
return (v->set_fn(v, &val));
}
-int owl_variable_enum_get_tostring(const owl_variable *v, char* buf, int bufsize, const void *val) {
+char *owl_variable_enum_get_tostring(const owl_variable *v, const void *val) {
char **enums;
int nenums, i;
+ char *tostring;
if (val == NULL) {
- snprintf(buf, bufsize, "<null>");
- return -1;
+ return g_strdup("<null>");
}
enums = g_strsplit_set(v->validsettings, ",", 0);
nenums = g_strv_length(enums);
i = *(const int*)val;
if (i<0 || i>=nenums) {
- snprintf(buf, bufsize, "<invalid:%d>",i);
g_strfreev(enums);
- return(-1);
+ return g_strdup_printf("<invalid:%d>", i);
}
- snprintf(buf, bufsize, "%s", enums[i]);
+ tostring = g_strdup(enums[i]);
g_strfreev(enums);
- return 0;
+ return tostring;
}
/* default functions for stringeans */
@@ -1016,13 +1008,11 @@ int owl_variable_string_set_fromstring_default(owl_variable *v, const char *newv
return (v->set_fn(v, newval));
}
-int owl_variable_string_get_tostring_default(const owl_variable *v, char* buf, int bufsize, const void *val) {
+char *owl_variable_string_get_tostring_default(const owl_variable *v, const void *val) {
if (val == NULL) {
- snprintf(buf, bufsize, "<null>");
- return -1;
+ return g_strdup("<null>");
} else {
- snprintf(buf, bufsize, "%s", (const char*)val);
- return 0;
+ return g_strdup((const char*)val);
}
}
--
1.7.3.3.399.g06ddf