From bb1f857b00e2e6a0fc6bb7d4b768e9f2ae37c730 Mon Sep 17 00:00:00 2001
From: Tony Crisci <tony@dubstepdish.com>
Date: Mon, 10 Nov 2014 00:04:47 -0500
Subject: [PATCH] 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.
---
 include/bindings.h                       |  5 +++
 src/bindings.c                           | 35 ++++++++++++++++--
 src/config.c                             |  4 +--
 testcases/t/238-regress-reload-bindsym.t | 45 ++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 6 deletions(-)
 create mode 100644 testcases/t/238-regress-reload-bindsym.t

diff --git a/include/bindings.h b/include/bindings.h
index e2acc313..02c3f190 100644
--- 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 b75c6357..0bda6019 100644
--- 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 5089ef22..b41f0e1b 100644
--- 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
index 00000000..6d5d12c3
--- /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;
-- 
2.39.5