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;