bugfix: copy binding before run

Copy the binding struct before running it and use this copy to emit the
binding event.

This fixes a crash when the command `reload` is used in a binding when
the binding event is emitted.

diff --git a/include/bindings.h b/include/bindings.h
index e2acc313485a805898a90ccac2ba922e858db2fb..02c3f190c9a8d7c0f8933eda4ea1f3806ac20aa2 100644 (file)
--- a/include/bindings.h
+++ b/include/bindings.h
@@ -60,6 +60,11 @@ void switch_mode(const char *new_mode);
  */
 void check_for_duplicate_bindings(struct context *context);
 
+/**
+ * Frees the binding. If bind is null, it simply returns.
+ */
+void binding_free(Binding *bind);
+
 /**
  * Runs the given binding and handles parse errors. If con is passed, it will
  * execute the command binding with that container selected by criteria.

diff --git a/src/bindings.c b/src/bindings.c
index b75c635728950bad989cda47cac80a7f4bf003ed..0bda6019187f6f1916ac93e1b86d27aaa079384f 100644 (file)
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -380,6 +380,33 @@ void check_for_duplicate_bindings(struct context *context) {
     }
 }
 
+/*
+ * Creates a dynamically allocated copy of bind.
+ */
+static Binding *binding_copy(Binding *bind) {
+    Binding *ret = smalloc(sizeof(Binding));
+    *ret = *bind;
+    ret->symbol = strdup(bind->symbol);
+    ret->command = strdup(bind->command);
+    ret->translated_to = smalloc(sizeof(xcb_keycode_t) * bind->number_keycodes);
+    memcpy(ret->translated_to, bind->translated_to, sizeof(xcb_keycode_t) * bind->number_keycodes);
+    return ret;
+}
+
+/*
+ * Frees the binding. If bind is null, it simply returns.
+ */
+void binding_free(Binding *bind) {
+    if (bind == NULL) {
+        return;
+    }
+
+    FREE(bind->symbol);
+    FREE(bind->translated_to);
+    FREE(bind->command);
+    FREE(bind);
+}
+
 /*
  * Runs the given binding and handles parse errors. If con is passed, it will
  * execute the command binding with that container selected by criteria.
@@ -390,14 +417,15 @@ void check_for_duplicate_bindings(struct context *context) {
 CommandResult *run_binding(Binding *bind, Con *con) {
     char *command;
 
-    /* We need to copy the command since “reload” may be part of the command,
-     * and then the memory that bind->command points to may not contain the
+    /* We need to copy the binding and command since “reload” may be part of
+     * the command, and then the memory that bind points to may not contain the
      * same data anymore. */
     if (con == NULL)
         command = sstrdup(bind->command);
     else
         sasprintf(&command, "[con_id=\"%d\"] %s", con, bind->command);
 
+    Binding *bind_cp = binding_copy(bind);
     CommandResult *result = parse_command(command, NULL);
     free(command);
 
@@ -423,7 +451,8 @@ CommandResult *run_binding(Binding *bind, Con *con) {
         free(pageraction);
     }
 
-    ipc_send_binding_event("run", bind);
+    ipc_send_binding_event("run", bind_cp);
+    binding_free(bind_cp);
 
     return result;
 }

diff --git a/src/config.c b/src/config.c
index 5089ef2202b89a5eac0f48c1d00e78d770cf7d5b..b41f0e1b7eb2f909aaf7afe9141ddf8c586bc71f 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -147,9 +147,7 @@ void load_configuration(xcb_connection_t *conn, const char *override_configpath,
             while (!TAILQ_EMPTY(bindings)) {
                 bind = TAILQ_FIRST(bindings);
                 TAILQ_REMOVE(bindings, bind, bindings);
-                FREE(bind->translated_to);
-                FREE(bind->command);
-                FREE(bind);
+                binding_free(bind);
             }
             FREE(bindings);
             SLIST_REMOVE(&modes, mode, Mode, modes);

diff --git a/testcases/t/238-regress-reload-bindsym.t b/testcases/t/238-regress-reload-bindsym.t
new file mode 100644 (file)
index 0000000..6d5d12c
--- /dev/null
+++ b/testcases/t/238-regress-reload-bindsym.t
@@ -0,0 +1,45 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • http://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • http://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • http://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Test that the binding event works properly
+# Ticket: #1210
+use i3test i3_autostart => 0;
+
+my $config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+bindsym r reload
+EOT
+
+SKIP: {
+    qx(which xdotool 2> /dev/null);
+
+    skip 'xdotool is required to test the binding event. `[apt-get install|pacman -S] xdotool`', 1 if $?;
+
+    my $pid = launch_with_config($config);
+
+    my $i3 = i3(get_socket_path());
+    $i3->connect->recv;
+
+    qx(xdotool key r);
+
+    does_i3_live;
+
+    exit_gracefully($pid);
+
+}
+done_testing;