On 16.07.2012 16:32, Tiago Vignatti wrote:
we were using wrong iterator for xcb_render_pictforminfo_t type, the
formats_reply->length; valgrind was shouting it loudly. Another issue this
patch addresses is that now find_depth returns the first util and valid format
that matches the desired depth; it doesn't continue through the loop until the
end.
This reverts part of commit 5ea11b69.
Signed-off-by: Tiago Vignatti <[email protected]>
---
src/xwayland/window-manager.c | 49 +++++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index 6e032ea..b922e9b 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -963,6 +963,37 @@ weston_wm_handle_event(int fd, uint32_t mask, void *data)
return count;
}
+static xcb_render_pictforminfo_t *
+find_depth (xcb_connection_t *c, int depth)
+{
+ xcb_render_query_pict_formats_reply_t *formats;
+ xcb_render_query_pict_formats_cookie_t cookie;
+ xcb_render_pictforminfo_iterator_t i;
+
+ cookie = xcb_render_query_pict_formats (c);
+ xcb_flush (c);
This call to xcb_flush() is unnecessary. XCB will send the request in
..._reply() if it wasn't sent yet.
+ formats = xcb_render_query_pict_formats_reply (c, cookie, 0);
+ if (formats == NULL)
+ return NULL;
+
+ for (i = xcb_render_query_pict_formats_formats_iterator (formats);
+ i.rem;
+ xcb_render_pictforminfo_next (&i))
+ {
+ if (XCB_RENDER_PICT_TYPE_DIRECT != i.data->type)
+ continue;
+
+ if (depth != i.data->depth)
+ continue;
+
+ return i.data;
Doesn't this leak 'formats'?
+ }
+
+ free (formats);
+ return NULL;
+}
+
static void
wxs_wm_get_resources(struct weston_wm *wm)
{
@@ -1024,15 +1055,10 @@ wxs_wm_get_resources(struct weston_wm *wm)
xcb_xfixes_query_version_reply_t *xfixes_reply;
xcb_intern_atom_cookie_t cookies[ARRAY_LENGTH(atoms)];
xcb_intern_atom_reply_t *reply;
- xcb_render_query_pict_formats_reply_t *formats_reply;
- xcb_render_query_pict_formats_cookie_t formats_cookie;
- xcb_render_pictforminfo_t *formats;
uint32_t i;
xcb_prefetch_extension_data (wm->conn, &xcb_xfixes_id);
- formats_cookie = xcb_render_query_pict_formats(wm->conn);
-
for (i = 0; i < ARRAY_LENGTH(atoms); i++)
cookies[i] = xcb_intern_atom (wm->conn, 0,
strlen(atoms[i].name),
@@ -1059,18 +1085,7 @@ wxs_wm_get_resources(struct weston_wm *wm)
free(xfixes_reply);
- formats_reply = xcb_render_query_pict_formats_reply(wm->conn,
- formats_cookie, 0);
- if (formats_reply == NULL)
- return;
-
- formats = xcb_render_query_pict_formats_formats(formats_reply);
- for (i = 0; i < formats_reply->length; i++)
- if (formats[i].type == XCB_RENDER_PICT_TYPE_DIRECT &&
- formats[i].depth == 24)
- wm->render_format = formats[i];
-
- free(formats_reply);
+ wm->render_format = *(find_depth(wm->conn, 24));
This smells like a NULL pointer dereference, some error handling would be nice.
}
static void
Also (this is not specific to this patch, the old code had this, too), what
happens when the X11 server doesn't have the RENDER extension? The code doesn't
seem to check for its existence (nor does it check its version). So I guess
right now libxcb would go into an error state without any kind of remotely
helpful error message.
Cheers,
Uli
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
