[2958] in BarnOwl Developers
[PATCH 1/2] Make owl_keybinding new/delete instead of init/cleanup
daemon@ATHENA.MIT.EDU (David Benjamin)
Thu Mar 10 15:49:07 2011
From: David Benjamin <davidben@MIT.EDU>
To: barnowl-dev@mit.edu
Date: Thu, 10 Mar 2011 15:48:42 -0500
Valgrind is deeply disappointed in this code...
Hide owl_keybinding_make_keys again, so we don't construct half-baked
owl_keybinding instances. Instead allow for creating a dummy keybinding.
Also, initialize all fields of owl_keybinding, or freeing it afterwards
explodes. Also fix a memory leak in unbindkey.
---
keybinding.c | 31 ++++++++++++++++---------------
keymap.c | 18 ++++++++----------
owl.h | 1 +
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/keybinding.c b/keybinding.c
index 3b15cd9..a5c977f 100644
--- a/keybinding.c
+++ b/keybinding.c
@@ -10,30 +10,37 @@
*
*/
+static int owl_keybinding_make_keys(owl_keybinding *kb, const char *keyseq);
+
/* sets up a new keybinding for a command */
-int owl_keybinding_init(owl_keybinding *kb, const char *keyseq, const char *command, void (*function_fn)(void), const char *desc)
+owl_keybinding *owl_keybinding_new(const char *keyseq, const char *command, void (*function_fn)(void), const char *desc)
{
+ owl_keybinding *kb = g_new0(owl_keybinding, 1);
+
owl_function_debugmsg("owl_keybinding_init: creating binding for <%s> with desc: <%s>", keyseq, desc);
- if (command && !function_fn) {
+ if (command && function_fn) {
+ g_free(kb);
+ return NULL;
+ } else if (command && !function_fn) {
kb->type = OWL_KEYBINDING_COMMAND;
} else if (!command && function_fn) {
kb->type = OWL_KEYBINDING_FUNCTION;
} else {
- return(-1);
+ kb->type = OWL_KEYBINDING_NOOP;
}
if (owl_keybinding_make_keys(kb, keyseq) != 0) {
- return(-1);
+ g_free(kb);
+ return NULL;
}
if (command) kb->command = g_strdup(command);
kb->function_fn = function_fn;
if (desc) kb->desc = g_strdup(desc);
- else kb->desc = NULL;
- return(0);
+ return kb;
}
-int owl_keybinding_make_keys(owl_keybinding *kb, const char *keyseq)
+static int owl_keybinding_make_keys(owl_keybinding *kb, const char *keyseq)
{
char **ktokens;
int nktokens, i;
@@ -58,18 +65,12 @@ int owl_keybinding_make_keys(owl_keybinding *kb, const char *keyseq)
return(0);
}
-/* Releases data associated with a keybinding */
-void owl_keybinding_cleanup(owl_keybinding *kb)
+/* Releases data associated with a keybinding, and the kb itself */
+void owl_keybinding_delete(owl_keybinding *kb)
{
if (kb->keys) g_free(kb->keys);
if (kb->desc) g_free(kb->desc);
if (kb->command) g_free(kb->command);
-}
-
-/* Releases data associated with a keybinding, and the kb itself */
-void owl_keybinding_delete(owl_keybinding *kb)
-{
- owl_keybinding_cleanup(kb);
g_free(kb);
}
diff --git a/keymap.c b/keymap.c
index fdf8c65..13a07dc 100644
--- a/keymap.c
+++ b/keymap.c
@@ -37,11 +37,9 @@ int owl_keymap_create_binding(owl_keymap *km, const char *keyseq, const char *co
owl_keybinding *kb, *curkb;
int i;
- if ((kb = g_new(owl_keybinding, 1)) == NULL) return(-1);
- if (0 != owl_keybinding_init(kb, keyseq, command, function_fn, desc)) {
- g_free(kb);
- return(-1);
- }
+ kb = owl_keybinding_new(keyseq, command, function_fn, desc);
+ if (kb == NULL)
+ return -1;
/* see if another matching binding, and if so remove it.
* otherwise just add this one.
*/
@@ -62,20 +60,20 @@ int owl_keymap_remove_binding(owl_keymap *km, const char *keyseq)
owl_keybinding *kb, *curkb;
int i;
- if ((kb = g_new(owl_keybinding, 1)) == NULL) return(-1);
- if (0 != owl_keybinding_make_keys(kb, keyseq)) {
- g_free(kb);
- return(-1);
- }
+ kb = owl_keybinding_new(keyseq, NULL, NULL, NULL);
+ if (kb == NULL)
+ return -1;
for (i = owl_list_get_size(&km->bindings)-1; i >= 0; i--) {
curkb = owl_list_get_element(&km->bindings, i);
if (owl_keybinding_equal(curkb, kb)) {
owl_list_remove_element(&km->bindings, i);
owl_keybinding_delete(curkb);
+ owl_keybinding_delete(kb);
return(0);
}
}
+ owl_keybinding_delete(kb);
return(-2);
}
diff --git a/owl.h b/owl.h
index 96640c6..56710ab 100644
--- a/owl.h
+++ b/owl.h
@@ -185,6 +185,7 @@ typedef void HV;
#define OWL_KEYMAP_MAXSTACK 20
+#define OWL_KEYBINDING_NOOP 0 /* dummy binding that does nothing */
#define OWL_KEYBINDING_COMMAND 1 /* command string */
#define OWL_KEYBINDING_FUNCTION 2 /* function taking no args */
--
1.7.3.3.399.g06ddf
