i3 - improved tiling WM

bugfix: copy binding before run

Patch status: merged

Patch by Tony Crisci

Long description:

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.

To apply this patch, use:
curl http://cr.i3wm.org/patch/668/raw.patch | git am

b/include/bindings.h

22 
@@ -61,6 +61,11 @@ void switch_mode(const char *new_mode);
23 
 void check_for_duplicate_bindings(struct context *context);
24
25 
 /**
26 
+ * Frees the binding. If bind is null, it simply returns.
27 
+ */
28 
+void binding_free(Binding *bind);
29 
+
30 
+/**
31 
  * Runs the given binding and handles parse errors. If con is passed, it will
32 
  * execute the command binding with that container selected by criteria.
33 
  * Returns a CommandResult for running the binding's command. Caller should

b/src/bindings.c

38 
@@ -381,6 +381,32 @@ void check_for_duplicate_bindings(struct context *context) {
39 
 }
40
41 
 /*
42 
+ * Creates a dynamically allocated copy of bind.
43 
+ */
44 
+static Binding *binding_copy(Binding *bind) {
45 
+    Binding *ret = smalloc(sizeof(Binding));
46 
+    *ret = *bind;
47 
+    ret->symbol = strdup(bind->symbol);
48 
+    ret->command = strdup(bind->command);
49 
+    ret->translated_to = smalloc(sizeof(xcb_keycode_t) * bind->number_keycodes);
50 
+    memcpy(ret->translated_to, bind->translated_to, sizeof(xcb_keycode_t) * bind->number_keycodes);
51 
+    return ret;
52 
+}
53 
+
54 
+/*
55 
+ * Frees the binding. If bind is null, it simply returns.
56 
+ */
57 
+void binding_free(Binding *bind) {
58 
+    if (bind == NULL) {
59 
+        return;
60 
+    }
61 
+
62 
+    FREE(bind->translated_to);
63 
+    FREE(bind->command);
64 
+    FREE(bind);
65 
+}
66 
+
67 
+/*
68 
  * Runs the given binding and handles parse errors. If con is passed, it will
69 
  * execute the command binding with that container selected by criteria.
70 
  * Returns a CommandResult for running the binding's command. Caller should
71 
@@ -390,14 +416,15 @@ void check_for_duplicate_bindings(struct context *context) {
72 
 CommandResult *run_binding(Binding *bind, Con *con) {
73 
     char *command;
74
75 
-    /* We need to copy the command since “reload” may be part of the command,
76 
-     * and then the memory that bind->command points to may not contain the
77 
+    /* We need to copy the binding and command since “reload” may be part of
78 
+     * the command, and then the memory that bind points to may not contain the
79 
      * same data anymore. */
80 
     if (con == NULL)
81 
         command = sstrdup(bind->command);
82 
     else
83 
         sasprintf(&command, "[con_id=\"%d\"] %s", con, bind->command);
84
85 
+    Binding *bind_cp = binding_copy(bind);
86 
     CommandResult *result = parse_command(command, NULL);
87 
     free(command);
88
89 
@@ -423,7 +450,8 @@ CommandResult *run_binding(Binding *bind, Con *con) {
90 
         free(pageraction);
91 
     }
92
93 
-    ipc_send_binding_event("run", bind);
94 
+    ipc_send_binding_event("run", bind_cp);
95 
+    binding_free(bind_cp);
96
97 
     return result;
98 
 }

b/src/config.c

103 
@@ -147,9 +147,7 @@ void load_configuration(xcb_connection_t *conn, const char *override_configpath,
104 
             while (!TAILQ_EMPTY(bindings)) {
105 
                 bind = TAILQ_FIRST(bindings);
106 
                 TAILQ_REMOVE(bindings, bind, bindings);
107 
-                FREE(bind->translated_to);
108 
-                FREE(bind->command);
109 
-                FREE(bind);
110 
+                binding_free(bind);
111 
             }
112 
             FREE(bindings);
113 
             SLIST_REMOVE(&modes, mode, Mode, modes);

b/testcases/t/238-regress-reload-bindsym.t

119 
@@ -0,0 +1,45 @@
120 
+#!perl
121 
+# vim:ts=4:sw=4:expandtab
122 
+#
123 
+# Please read the following documents before working on tests:
124 
+# • http://build.i3wm.org/docs/testsuite.html
125 
+#   (or docs/testsuite)
126 
+#
127 
+# • http://build.i3wm.org/docs/lib-i3test.html
128 
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
129 
+#
130 
+# • http://build.i3wm.org/docs/ipc.html
131 
+#   (or docs/ipc)
132 
+#
133 
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
134 
+#   (unless you are already familiar with Perl)
135 
+#
136 
+# Test that the binding event works properly
137 
+# Ticket: #1210
138 
+use i3test i3_autostart => 0;
139 
+
140 
+my $config = <<EOT;
141 
+# i3 config file (v4)
142 
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
143 
+
144 
+bindsym r reload
145 
+EOT
146 
+
147 
+SKIP: {
148 
+    qx(which xdotool 2> /dev/null);
149 
+
150 
+    skip 'xdotool is required to test the binding event. `[apt-get install|pacman -S] xdotool`', 1 if $?;
151 
+
152 
+    my $pid = launch_with_config($config);
153 
+
154 
+    my $i3 = i3(get_socket_path());
155 
+    $i3->connect->recv;
156 
+
157 
+    qx(xdotool key r);
158 
+
159 
+    does_i3_live;
160 
+
161 
+    exit_gracefully($pid);
162 
+
163 
+}
164 
+done_testing;