[Xcb] [RFC libxcb 0/2] full sequence for everyone

Daniel Martin consume.noise at gmail.com
Sun Feb 17 05:54:38 PST 2013 

Hi,

these 2 patches are an attempt to fix the issue of the full_sequence
field in combination with X Generic Events. I haven't tested them
excessively, before doing this I would like to get some feedback what
you think about this approach.

From the API/ABI side I've removed the full_sequence from xcb_ge_event_t
because an XGE may be bigger then 32bytes. Having the full_sequence in
between the event data is possible from a practical pov, but imho it's
not nice to inject additional data. That's the first patch:
      Remove full_sequence from xcb_ge_event_t

The second patch
      Add full sequence to every reply
attaches the full sequence to every incoming reply. Before this patch
the full sequence was added to
    - errors and
    - events only.
Whereas it could overwrite event data for XGEs > 32bytes.

Now, the full sequence will be _appended_ to every incoming reply. That
are:
    - errors,
    - events and
    - replies to requests.
As I didn't changed any data structure the field is somewhat hidden -
after the reply. Therefor I've added a function
        xcb_get_full_sequence()
to retrieve it in a convinient way.


At least, I'm unhappy with the description of the function. The rest
should do the trick.


Thanks for your feedback,
    Daniel Martin

PS: Didn't changed that much:
 src/xcb.h    | 11 ++++++++++-
 src/xcb_in.c | 27 +++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 5 deletions(-)
-------------- next part --------------
From 53141bc0ac15cde19bfcd3b57f75d484282d818b Mon Sep 17 00:00:00 2001
From: Daniel Martin <consume.noise at gmail.com>
Date: Sun, 17 Feb 2013 01:41:29 +0100
Subject: [PATCH libxcb 1/2] Remove full_sequence from xcb_ge_event_t

X Generic Events may be bigger then 32bytes. So, there might be event
data behind the 32byte boundary and therefor we would overwrite event
data if we set the full_sequence there.

The full_sequence field will be added dynamically to the end of the
specific event itself with the next commit.
---
 src/xcb.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/xcb.h b/src/xcb.h
index f7dc6af..6fc3be2 100644
--- a/src/xcb.h
+++ b/src/xcb.h
@@ -151,7 +151,6 @@ typedef struct {
     uint16_t event_type;
     uint16_t pad1;
     uint32_t pad[5];         /**< Padding */
-    uint32_t full_sequence;  /**< full sequence */
 } xcb_ge_event_t;
 
 /**
-- 
1.8.1.3

-------------- next part --------------
From 645cd8c5ec14f1a054666bd99848e3f7dfd0ce8e Mon Sep 17 00:00:00 2001
From: Daniel Martin <consume.noise at gmail.com>
Date: Sun, 17 Feb 2013 14:08:17 +0100
Subject: [PATCH libxcb 2/2] Add full sequence to every reply

With this commit the full sequence will be attached to every incoming
reply from the server. Which could be an event (including X Generic
Events), error or reply to a request. As the data structures itself
haven't been modified, a function has been added:
    xcb_get_full_sequence()
to get the full sequence in a convenient way.

Before this commit, the full sequence was attached to events and errors
only and XGEs had been a problem.
---
 src/xcb.h    | 10 ++++++++++
 src/xcb_in.c | 27 +++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/xcb.h b/src/xcb.h
index 6fc3be2..db6432c 100644
--- a/src/xcb.h
+++ b/src/xcb.h
@@ -264,6 +264,16 @@ void xcb_prefetch_maximum_request_length(xcb_connection_t *c);
 /* xcb_in.c */
 
 /**
+ * @brief Return the full sequence of a reply, event or error.
+ * @param d: The reply, event or error.
+ * @return The full sequence of a reply, event or error.
+ *
+ * Returns the full sequence of a reply, event or error, which is
+ * attached to the structure.
+ */
+uint32_t xcb_get_full_sequence(void *d);
+
+/**
  * @brief Returns the next event or error from the server.
  * @param c: The connection to the X server.
  * @return The next event from the server.
diff --git a/src/xcb_in.c b/src/xcb_in.c
index b810783..5c4046b 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -90,12 +90,18 @@ static void remove_finished_readers(reader_list **prev_reader, uint64_t complete
     }
 }
 
+static uint32_t *get_full_sequence_ptr(void *d, int length)
+{
+    return (uint32_t*)((char*)d + length);
+}
+
 static int read_packet(xcb_connection_t *c)
 {
     xcb_generic_reply_t genrep;
     int length = 32;
     int eventlength = 0; /* length after first 32 bytes for GenericEvents */
     void *buf;
+    uint32_t *full_sequence;
     pending_reply *pend = 0;
     struct event_list *event;
 
@@ -169,8 +175,8 @@ static int read_packet(xcb_connection_t *c)
     if ((genrep.response_type & 0x7f) == XCB_XGE_EVENT)
         eventlength = genrep.length * 4;
 
-    buf = malloc(length + eventlength +
-            (genrep.response_type == XCB_REPLY ? 0 : sizeof(uint32_t)));
+    buf = malloc(length + eventlength + sizeof(uint32_t));
+    full_sequence = get_full_sequence_ptr(buf, length + eventlength);
     if(!buf)
     {
         _xcb_conn_shutdown(c, XCB_CONN_CLOSED_MEM_INSUFFICIENT);
@@ -199,8 +205,7 @@ static int read_packet(xcb_connection_t *c)
         return 1;
     }
 
-    if(genrep.response_type != XCB_REPLY)
-        ((xcb_generic_event_t *) buf)->full_sequence = c->in.request_read;
+    *full_sequence = c->in.request_read;
 
     /* reply, or checked error */
     if( genrep.response_type == XCB_REPLY ||
@@ -412,6 +417,20 @@ static uint64_t widen(xcb_connection_t *c, unsigned int request)
 
 /* Public interface */
 
+uint32_t xcb_get_full_sequence(void *d)
+{
+    xcb_generic_reply_t *genrep = d;
+    int length = 32;
+
+    if((genrep->response_type == XCB_REPLY) ||
+       ((genrep->response_type & 0x7f) == XCB_XGE_EVENT))
+    {
+        length += genrep->length * 4;
+    }
+
+    return *get_full_sequence_ptr(d, length);
+}
+
 void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_error_t **e)
 {
     void *ret;
-- 
1.8.1.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130217/31b18b31/attachment.pgp>

More information about the Xcb mailing list