i3 - improved tiling WM

Reviews
Bugs

Add xcb_get_property_unchecked_no_length wrapper

Patch status: rejected

Patch by Lancelot SIX

Long description:

Add a wrapper to xcb_get_property_unchecked in order remove the
necessity to know in advance the lenth of a X property. This is usefull
when the length of the property is not known in advance.
The wrapper requires the appropriate amount of data when requesting a property
from X.

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

b/i3bar/src/xcb.c

@@ -960,24 +960,19 @@ char *init_xcb_early() {
     /* Now we get the atoms and save them in a nice data structure */
     get_atoms();
 
-    xcb_get_property_cookie_t path_cookie;
-    path_cookie = xcb_get_property_unchecked(xcb_connection,
-                                   0,
+    /* We check, if i3 set its socket-path */
+    xcb_get_property_reply_t *path_reply =
+        xcb_get_property_unchecked_no_length(xcb_connection,
                                    xcb_root,
                                    atoms[I3_SOCKET_PATH],
-                                   XCB_GET_PROPERTY_TYPE_ANY,
-                                   0, PATH_MAX);
-
-    /* We check, if i3 set its socket-path */
-    xcb_get_property_reply_t *path_reply = xcb_get_property_reply(xcb_connection,
-                                                                  path_cookie,
-                                                                  NULL);
+                                   XCB_GET_PROPERTY_TYPE_ANY);
     char *path = NULL;
     if (path_reply) {
         int len = xcb_get_property_value_length(path_reply);
         if (len != 0) {
             path = strndup(xcb_get_property_value(path_reply), len);
         }
+        free(path_reply);
     }
 
 

b/include/libi3.h

@@ -91,6 +91,19 @@ void errorlog(char *fmt, ...);
 char *root_atom_contents(const char *atomname);
 
 /**
+ * Wrapper around xcb_get_property_unchecked where the length of the property
+ * does not need to be specified. The wrapper specifies a size sufficient to
+ * return the property.
+ *
+ * Returned value must be freed by the caller
+ */
+xcb_get_property_reply_t *
+xcb_get_property_unchecked_no_length(xcb_connection_t *conn,
+                                     xcb_window_t window,
+                                     xcb_atom_t property,
+                                     xcb_atom_t type);
+
+/**
  * Safe-wrapper around malloc which exits if malloc returns NULL (meaning that
  * there is no more memory available)
  *

b/libi3/root_atom_contents.c

@@ -9,6 +9,7 @@
 #include <string.h>
 #include <stdbool.h>
 #include <limits.h>
+#include <stdlib.h>
 
 #include <xcb/xcb.h>
 #include <xcb/xcb_aux.h>
@@ -43,24 +44,27 @@ char *root_atom_contents(const char *atomname) {
     if (atom_reply == NULL)
         return NULL;
 
-    xcb_get_property_cookie_t prop_cookie;
-    xcb_get_property_reply_t *prop_reply;
-    prop_cookie = xcb_get_property_unchecked(conn, false, root, atom_reply->atom,
-                                             XCB_GET_PROPERTY_TYPE_ANY, 0, PATH_MAX);
-    prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
+    xcb_get_property_reply_t *prop_reply =
+        xcb_get_property_unchecked_no_length(conn, root,atom_reply->atom,
+                                             XCB_GET_PROPERTY_TYPE_ANY);
     if (prop_reply == NULL || xcb_get_property_value_length(prop_reply) == 0)
         return NULL;
     if (prop_reply->type == XCB_ATOM_CARDINAL) {
         /* We treat a CARDINAL as a >= 32-bit unsigned int. The only CARDINAL
          * we query is I3_PID, which is 32-bit. */
-        if (asprintf(&content, "%u", *((unsigned int*)xcb_get_property_value(prop_reply))) == -1)
+        if (asprintf(&content, "%u", *((unsigned int*)xcb_get_property_value(prop_reply))) == -1) {
+            free(prop_reply);
             return NULL;
+        }
     } else {
         if (asprintf(&content, "%.*s", xcb_get_property_value_length(prop_reply),
-                     (char*)xcb_get_property_value(prop_reply)) == -1)
+                     (char*)xcb_get_property_value(prop_reply)) == -1) {
+            free(prop_reply);
             return NULL;
+        }
     }
     xcb_disconnect(conn);
+    free(prop_reply);
     return content;
 }
 

b/libi3/xcb_get_property_unchecked_no_length.c

@@ -0,0 +1,53 @@
+/*
+ * vim:ts=4:sw=4:expandtab
+ *
+ * i3 - an improved dynamic tiling window manager
+ * © 2009-2011 Michael Stapelberg and contributors (see also: LICENSE)
+ *
+ */
+#include <stdlib.h>
+#include <xcb/xcb.h>
+#include "libi3.h"
+
+/**
+ * Wrapper around xcb_get_property_unchecked where the length of the property
+ * does not need to be specified. The wrapper specifies a size sufficient to
+ * return the property.
+ *
+ * Returned value must be freed by the caller
+ */
+xcb_get_property_reply_t *
+xcb_get_property_unchecked_no_length(xcb_connection_t *conn,
+                                     xcb_window_t window,
+                                     xcb_atom_t property,
+                                     xcb_atom_t type) {
+    xcb_get_property_cookie_t prop_cookie;
+    xcb_get_property_reply_t *prop_reply;
+    /* xcb_get_property_unsafe needs the size of the required property
+     * expressed in words (4 bytes). Therefor, prop_max_size should always
+     * be a multiple of 4. */
+    size_t prop_max_size = 1024;
+    size_t prop_size;
+
+    prop_cookie = xcb_get_property_unchecked(conn, false, window, property,
+                                             type, 0, (prop_max_size / 4));
+    prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
+    if (prop_reply == NULL)
+        return NULL;
+
+    prop_size = xcb_get_property_value_length(prop_reply);
+    if (prop_size > 0 && prop_reply->bytes_after) {
+        /* We received an incomplete value. Ask again but with a properly
+         * adjusted size. */
+        prop_max_size += prop_reply->bytes_after;
+        if ((prop_max_size % 4) != 0)
+            prop_max_size += 4 - (prop_max_size % 4);
+
+        /* Repeat the request */
+        free(prop_reply);
+        prop_cookie = xcb_get_property_unchecked(conn, false, window, property,
+                                                 type, 0, (prop_max_size / 4));
+        prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
+    }
+    return prop_reply;
+}

56	@@ -91,6 +91,19 @@ void errorlog(char *fmt, ...);
57	char root_atom_contents(const char atomname);
58
59	/**
60	+ * Wrapper around xcb_get_property_unchecked where the length of the property
61	+ * does not need to be specified. The wrapper specifies a size sufficient to
62	+ * return the property.
63	+ *
64	+ * Returned value must be freed by the caller
65	+ */
66	+xcb_get_property_reply_t *
67	+xcb_get_property_unchecked_no_length(xcb_connection_t *conn,
68	+ xcb_window_t window,
69	+ xcb_atom_t property,
70	+ xcb_atom_t type);
71	+
72	+/**
73	* Safe-wrapper around malloc which exits if malloc returns NULL (meaning that
74	* there is no more memory available)
75	*

128	@@ -0,0 +1,53 @@
129	+/*
130	+ * vim:ts=4:sw=4:expandtab
131	+ *
132	+ * i3 - an improved dynamic tiling window manager
133	+ * © 2009-2011 Michael Stapelberg and contributors (see also: LICENSE)
134	+ *
135	+ */
136	+#include <stdlib.h>
137	+#include <xcb/xcb.h>
138	+#include "libi3.h"
139	+
140	+/**
141	+ * Wrapper around xcb_get_property_unchecked where the length of the property
142	+ * does not need to be specified. The wrapper specifies a size sufficient to
143	+ * return the property.
144	+ *
145	+ * Returned value must be freed by the caller
146	+ */
147	+xcb_get_property_reply_t *
148	+xcb_get_property_unchecked_no_length(xcb_connection_t *conn,
149	+ xcb_window_t window,
150	+ xcb_atom_t property,
151	+ xcb_atom_t type) {
152	+ xcb_get_property_cookie_t prop_cookie;
153	+ xcb_get_property_reply_t *prop_reply;
154	+ /* xcb_get_property_unsafe needs the size of the required property
155	+ * expressed in words (4 bytes). Therefor, prop_max_size should always
156	+ * be a multiple of 4. */
157	+ size_t prop_max_size = 1024;
158	+ size_t prop_size;
159	+
160	+ prop_cookie = xcb_get_property_unchecked(conn, false, window, property,
161	+ type, 0, (prop_max_size / 4));
162	+ prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
163	+ if (prop_reply == NULL)
164	+ return NULL;
165	+
166	+ prop_size = xcb_get_property_value_length(prop_reply);
167	+ if (prop_size > 0 && prop_reply->bytes_after) {
168	+ /* We received an incomplete value. Ask again but with a properly
169	+ * adjusted size. */
170	+ prop_max_size += prop_reply->bytes_after;
171	+ if ((prop_max_size % 4) != 0)
172	+ prop_max_size += 4 - (prop_max_size % 4);
173	+
174	+ /* Repeat the request */
175	+ free(prop_reply);
176	+ prop_cookie = xcb_get_property_unchecked(conn, false, window, property,
177	+ type, 0, (prop_max_size / 4));
178	+ prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
179	+ }
180	+ return prop_reply;
181	+}

22	@@ -960,24 +960,19 @@ char *init_xcb_early() {
23	/* Now we get the atoms and save them in a nice data structure */
24	get_atoms();
25
26	- xcb_get_property_cookie_t path_cookie;
27	- path_cookie = xcb_get_property_unchecked(xcb_connection,
28	- 0,
29	+ /* We check, if i3 set its socket-path */
30	+ xcb_get_property_reply_t *path_reply =
31	+ xcb_get_property_unchecked_no_length(xcb_connection,
32	xcb_root,
33	atoms[I3_SOCKET_PATH],
34	- XCB_GET_PROPERTY_TYPE_ANY,
35	- 0, PATH_MAX);
36	-
37	- /* We check, if i3 set its socket-path */
38	- xcb_get_property_reply_t *path_reply = xcb_get_property_reply(xcb_connection,
39	- path_cookie,
40	- NULL);
41	+ XCB_GET_PROPERTY_TYPE_ANY);
42	char *path = NULL;
43	if (path_reply) {
44	int len = xcb_get_property_value_length(path_reply);
45	if (len != 0) {
46	path = strndup(xcb_get_property_value(path_reply), len);
47	}
48	+ free(path_reply);
49	}
50
51

80	@@ -9,6 +9,7 @@
81	#include <string.h>
82	#include <stdbool.h>
83	#include <limits.h>
84	+#include <stdlib.h>
85
86	#include <xcb/xcb.h>
87	#include <xcb/xcb_aux.h>
88	@@ -43,24 +44,27 @@ char root_atom_contents(const char atomname) {
89	if (atom_reply == NULL)
90	return NULL;
91
92	- xcb_get_property_cookie_t prop_cookie;
93	- xcb_get_property_reply_t *prop_reply;
94	- prop_cookie = xcb_get_property_unchecked(conn, false, root, atom_reply->atom,
95	- XCB_GET_PROPERTY_TYPE_ANY, 0, PATH_MAX);
96	- prop_reply = xcb_get_property_reply(conn, prop_cookie, NULL);
97	+ xcb_get_property_reply_t *prop_reply =
98	+ xcb_get_property_unchecked_no_length(conn, root,atom_reply->atom,
99	+ XCB_GET_PROPERTY_TYPE_ANY);
100	if (prop_reply == NULL \|\| xcb_get_property_value_length(prop_reply) == 0)
101	return NULL;
102	if (prop_reply->type == XCB_ATOM_CARDINAL) {
103	/* We treat a CARDINAL as a >= 32-bit unsigned int. The only CARDINAL
104	* we query is I3_PID, which is 32-bit. */
105	- if (asprintf(&content, "%u", ((unsigned int)xcb_get_property_value(prop_reply))) == -1)
106	+ if (asprintf(&content, "%u", ((unsigned int)xcb_get_property_value(prop_reply))) == -1) {
107	+ free(prop_reply);
108	return NULL;
109	+ }
110	} else {
111	if (asprintf(&content, "%.*s", xcb_get_property_value_length(prop_reply),
112	- (char*)xcb_get_property_value(prop_reply)) == -1)
113	+ (char*)xcb_get_property_value(prop_reply)) == -1) {
114	+ free(prop_reply);
115	return NULL;
116	+ }
117	}
118	xcb_disconnect(conn);
119	+ free(prop_reply);
120	return content;
121	}
122