[2946] in BarnOwl Developers

home help back first fref pref prev next nref lref last post

[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


home help back first fref pref prev next nref lref last post