i3 - improved tiling WM

Reviews
Bugs

Authentication in different process

Patch status: rejected

Patch by Philippe Virouleau

Long description:

Add the possibility to buffer keystrokes when a process is running authentication
Fix #1090.

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

b/i3lock.1

@@ -107,6 +107,15 @@ another try. This can be useful if the XF86ScreenSaver key is used to
 put a laptop to sleep and bounce on resume or if you happen to wake up
 your computer with the enter key.
 
+.SH BUGS
+.B i3lock
+uses a buffer to store keystrokes events during authentication, if it is full
+the events are dropped. There is a known issue if you do hit the maximum size
+of the buffer : pressing shift + a letter as the last two events in the buffer
+will cause the shift release event to be dropped, therefore all the upcomming
+keystrokes will have the shift modifier. A work around is to use capslock to
+cancel the shift modifier.
+
 .SH SEE ALSO
 .IR xautolock(1)
 \- use i3lock as your screen saver

b/i3lock.c

@@ -63,6 +63,18 @@ cairo_surface_t *img = NULL;
 bool tile = false;
 bool ignore_empty_password = false;
 
+typedef struct ringbuffer_t {
+    int head;
+    int tail;
+    int size;
+    xcb_generic_event_t *buf[1024];
+} ringbuffer_t;
+/* Size of the ringbuffer */
+const int ringbuffer_max = 1024;
+
+/* Ring buffer where keystrokes are stored during authentication */
+static ringbuffer_t buffered_events = { 0, 0, 0 };
+
 /* isutf, u8_dec © 2005 Jeff Bezanson, public domain */
 #define isutf(c) (((c) & 0xC0) != 0x80)
 
@@ -75,6 +87,39 @@ void u8_dec(char *s, int *i) {
 }
 
 /*
+ * Add an event to the ring buffer.
+ *
+ * Return true if the event was added, false if there was an error
+ * (no space available or null buffer).
+ *
+ */
+static bool ringbuffer_add(ringbuffer_t *bufring, xcb_generic_event_t *event) {
+    if (bufring && bufring->size < ringbuffer_max) {
+        bufring->buf[bufring->head++] = event;
+        bufring->head %= ringbuffer_max;
+        bufring->size++;
+        return true;
+    }
+    return false;
+}
+
+/*
+ * Remove an event from the ring buffer.
+ *
+ * Return null if there was an error (empty or null buffer).
+ *
+ */
+static xcb_generic_event_t *ringbuffer_remove(ringbuffer_t *bufring) {
+    xcb_generic_event_t *ret_val = NULL;
+    if (bufring && bufring->size > 0) {
+        ret_val = bufring->buf[bufring->tail++];
+        bufring->tail %= ringbuffer_max;
+        bufring->size--;
+    }
+    return ret_val;
+}
+
+/*
  * Loads the XKB keymap from the X11 server and feeds it to xkbcommon.
  * Necessary so that we can properly let xkbcommon track the keyboard state and
  * translate keypresses to utf-8.
@@ -187,10 +232,14 @@ static void clear_pam_wrong(EV_P_ ev_timer *w, int revents) {
     clear_pam_wrong_timeout = NULL;
 }
 
-static void clear_input(void) {
+static void reset_input(void) {
     input_position = 0;
     clear_password_memory();
     password[input_position] = '\0';
+}
+
+static void clear_input(void) {
+    reset_input();
 
     /* Hide the unlock indicator after a bit if the password buffer is
      * empty. */
@@ -205,7 +254,6 @@ static void auth_failed(void) {
         fprintf(stderr, "Authentication failure\n");
 
     pam_state = STATE_PAM_WRONG;
-    clear_input();
     redraw_screen();
 
     /* Clear this state after 2 seconds (unless the user enters another
@@ -227,6 +275,9 @@ static void auth_failed(void) {
     }
 }
 
+static void handle_key_press(xcb_key_press_event_t *event);
+static void handle_key_release(xcb_key_release_event_t *event);
+
 static void child_cb(EV_P_ ev_child *child_watcher, int revents) {
     if (child_watcher->rstatus != 0) {
         DEBUG("Authentication successfull\n");
@@ -235,6 +286,17 @@ static void child_cb(EV_P_ ev_child *child_watcher, int revents) {
         exit(0);
     } else {
         auth_failed();
+        xcb_generic_event_t *event;
+        while (pam_state != STATE_PAM_VERIFY &&
+            (event = ringbuffer_remove(&buffered_events))) {
+            /* Strip off the highest bit (set if the event is generated) */
+            int type = (event->response_type & 0x7F);
+            if (type == XCB_KEY_PRESS)
+                handle_key_press((xcb_key_press_event_t*)event);
+            else if (type == XCB_KEY_RELEASE)
+                handle_key_release((xcb_key_release_event_t*)event);
+            free(event);
+        }
     }
     ev_child_stop(main_loop, child_watcher);
     free(child_watcher);
@@ -259,7 +321,8 @@ static void input_done(void) {
     if (cpid == 0) {
         exit(pam_authenticate(pam_handle, 0) == PAM_SUCCESS);
     } else if (cpid > 0) {
-        struct ev_child *child_watcher = calloc(sizeof(struct ev_io), 1);
+        reset_input();
+        struct ev_child *child_watcher = calloc(sizeof(struct ev_child), 1);
         ev_child_init(child_watcher, child_cb, cpid, 0);
         ev_child_set(child_watcher, cpid, 0);
         ev_child_start(EV_DEFAULT_ child_watcher);
@@ -281,6 +344,11 @@ static void input_done(void) {
  */
 static void handle_key_release(xcb_key_release_event_t *event) {
     xkb_state_update_key(xkb_state, event->detail, XKB_KEY_UP);
+
+    /* If this was the backspace or escape key we are back at an
+     * empty input, so turn off the screen if DPMS is enabled */
+    if (dpms && input_position == 0)
+        dpms_turn_off_screen(conn);
 }
 
 static void redraw_timeout(EV_P_ ev_timer *w, int revents) {
@@ -518,6 +586,14 @@ static void xcb_check_cb(EV_P_ ev_check *w, int revents) {
 
         /* Strip off the highest bit (set if the event is generated) */
         int type = (event->response_type & 0x7F);
+        if (pam_state == STATE_PAM_VERIFY &&
+            (type == XCB_KEY_PRESS || type == XCB_KEY_RELEASE)) {
+            /* Don’t process KeyPress/KeyRelease events when a verification is
+             * in progress. Try to buffer them or drop them if the buffer is full. */
+            if (!ringbuffer_add(&buffered_events, event))
+                free(event);
+            continue;
+        }
         switch (type) {
             case XCB_KEY_PRESS:
                 handle_key_press((xcb_key_press_event_t*)event);
@@ -525,12 +601,6 @@ static void xcb_check_cb(EV_P_ ev_check *w, int revents) {
 
             case XCB_KEY_RELEASE:
                 handle_key_release((xcb_key_release_event_t*)event);
-
-                /* If this was the backspace or escape key we are back at an
-                 * empty input, so turn off the screen if DPMS is enabled */
-                if (dpms && input_position == 0)
-                    dpms_turn_off_screen(conn);
-
                 break;
 
             case XCB_VISIBILITY_NOTIFY:

16	@@ -107,6 +107,15 @@ another try. This can be useful if the XF86ScreenSaver key is used to
17	put a laptop to sleep and bounce on resume or if you happen to wake up
18	your computer with the enter key.
19
20	+.SH BUGS
21	+.B i3lock
22	+uses a buffer to store keystrokes events during authentication, if it is full
23	+the events are dropped. There is a known issue if you do hit the maximum size
24	+of the buffer : pressing shift + a letter as the last two events in the buffer
25	+will cause the shift release event to be dropped, therefore all the upcomming
26	+keystrokes will have the shift modifier. A work around is to use capslock to
27	+cancel the shift modifier.
28	+
29	.SH SEE ALSO
30	.IR xautolock(1)
31	\- use i3lock as your screen saver

36	@@ -63,6 +63,18 @@ cairo_surface_t *img = NULL;
37	bool tile = false;
38	bool ignore_empty_password = false;
39
40	+typedef struct ringbuffer_t {
41	+ int head;
42	+ int tail;
43	+ int size;
44	+ xcb_generic_event_t *buf[1024];
45	+} ringbuffer_t;
46	+/* Size of the ringbuffer */
47	+const int ringbuffer_max = 1024;
48	+
49	+/* Ring buffer where keystrokes are stored during authentication */
50	+static ringbuffer_t buffered_events = { 0, 0, 0 };
51	+
52	/* isutf, u8_dec © 2005 Jeff Bezanson, public domain */
53	#define isutf(c) (((c) & 0xC0) != 0x80)
54
55	@@ -75,6 +87,39 @@ void u8_dec(char s, int i) {
56	}
57
58	/*
59	+ * Add an event to the ring buffer.
60	+ *
61	+ * Return true if the event was added, false if there was an error
62	+ * (no space available or null buffer).
63	+ *
64	+ */
65	+static bool ringbuffer_add(ringbuffer_t bufring, xcb_generic_event_t event) {
66	+ if (bufring && bufring->size < ringbuffer_max) {
67	+ bufring->buf[bufring->head++] = event;
68	+ bufring->head %= ringbuffer_max;
69	+ bufring->size++;
70	+ return true;
71	+ }
72	+ return false;
73	+}
74	+
75	+/*
76	+ * Remove an event from the ring buffer.
77	+ *
78	+ * Return null if there was an error (empty or null buffer).
79	+ *
80	+ */
81	+static xcb_generic_event_t ringbuffer_remove(ringbuffer_t bufring) {
82	+ xcb_generic_event_t *ret_val = NULL;
83	+ if (bufring && bufring->size > 0) {
84	+ ret_val = bufring->buf[bufring->tail++];
85	+ bufring->tail %= ringbuffer_max;
86	+ bufring->size--;
87	+ }
88	+ return ret_val;
89	+}
90	+
91	+/*
92	* Loads the XKB keymap from the X11 server and feeds it to xkbcommon.
93	* Necessary so that we can properly let xkbcommon track the keyboard state and
94	* translate keypresses to utf-8.
95	@@ -187,10 +232,14 @@ static void clear_pam_wrong(EV_P_ ev_timer *w, int revents) {
96	clear_pam_wrong_timeout = NULL;
97	}
98
99	-static void clear_input(void) {
100	+static void reset_input(void) {
101	input_position = 0;
102	clear_password_memory();
103	password[input_position] = '\0';
104	+}
105	+
106	+static void clear_input(void) {
107	+ reset_input();
108
109	/* Hide the unlock indicator after a bit if the password buffer is
110	* empty. */
111	@@ -205,7 +254,6 @@ static void auth_failed(void) {
112	fprintf(stderr, "Authentication failure\n");
113
114	pam_state = STATE_PAM_WRONG;
115	- clear_input();
116	redraw_screen();
117
118	/* Clear this state after 2 seconds (unless the user enters another
119	@@ -227,6 +275,9 @@ static void auth_failed(void) {
120	}
121	}
122
123	+static void handle_key_press(xcb_key_press_event_t *event);
124	+static void handle_key_release(xcb_key_release_event_t *event);
125	+
126	static void child_cb(EV_P_ ev_child *child_watcher, int revents) {
127	if (child_watcher->rstatus != 0) {
128	DEBUG("Authentication successfull\n");
129	@@ -235,6 +286,17 @@ static void child_cb(EV_P_ ev_child *child_watcher, int revents) {
130	exit(0);
131	} else {
132	auth_failed();
133	+ xcb_generic_event_t *event;
134	+ while (pam_state != STATE_PAM_VERIFY &&
135	+ (event = ringbuffer_remove(&buffered_events))) {
136	+ /* Strip off the highest bit (set if the event is generated) */
137	+ int type = (event->response_type & 0x7F);
138	+ if (type == XCB_KEY_PRESS)
139	+ handle_key_press((xcb_key_press_event_t*)event);
140	+ else if (type == XCB_KEY_RELEASE)
141	+ handle_key_release((xcb_key_release_event_t*)event);
142	+ free(event);
143	+ }
144	}
145	ev_child_stop(main_loop, child_watcher);
146	free(child_watcher);
147	@@ -259,7 +321,8 @@ static void input_done(void) {
148	if (cpid == 0) {
149	exit(pam_authenticate(pam_handle, 0) == PAM_SUCCESS);
150	} else if (cpid > 0) {
151	- struct ev_child *child_watcher = calloc(sizeof(struct ev_io), 1);
152	+ reset_input();
153	+ struct ev_child *child_watcher = calloc(sizeof(struct ev_child), 1);
154	ev_child_init(child_watcher, child_cb, cpid, 0);
155	ev_child_set(child_watcher, cpid, 0);
156	ev_child_start(EV_DEFAULT_ child_watcher);
157	@@ -281,6 +344,11 @@ static void input_done(void) {
158	*/
159	static void handle_key_release(xcb_key_release_event_t *event) {
160	xkb_state_update_key(xkb_state, event->detail, XKB_KEY_UP);
161	+
162	+ /* If this was the backspace or escape key we are back at an
163	+ * empty input, so turn off the screen if DPMS is enabled */
164	+ if (dpms && input_position == 0)
165	+ dpms_turn_off_screen(conn);
166	}
167
168	static void redraw_timeout(EV_P_ ev_timer *w, int revents) {
169	@@ -518,6 +586,14 @@ static void xcb_check_cb(EV_P_ ev_check *w, int revents) {
170
171	/* Strip off the highest bit (set if the event is generated) */
172	int type = (event->response_type & 0x7F);
173	+ if (pam_state == STATE_PAM_VERIFY &&
174	+ (type == XCB_KEY_PRESS \|\| type == XCB_KEY_RELEASE)) {
175	+ /* Don’t process KeyPress/KeyRelease events when a verification is
176	+ * in progress. Try to buffer them or drop them if the buffer is full. */
177	+ if (!ringbuffer_add(&buffered_events, event))
178	+ free(event);
179	+ continue;
180	+ }
181	switch (type) {
182	case XCB_KEY_PRESS:
183	handle_key_press((xcb_key_press_event_t*)event);
184	@@ -525,12 +601,6 @@ static void xcb_check_cb(EV_P_ ev_check *w, int revents) {
185
186	case XCB_KEY_RELEASE:
187	handle_key_release((xcb_key_release_event_t*)event);
188	-
189	- /* If this was the backspace or escape key we are back at an
190	- * empty input, so turn off the screen if DPMS is enabled */
191	- if (dpms && input_position == 0)
192	- dpms_turn_off_screen(conn);
193	-
194	break;
195
196	case XCB_VISIBILITY_NOTIFY: