[Xcb] XSync extension broken in many ways..
Barton C Massey
bart at cs.pdx.edu
Mon Aug 27 12:43:37 PDT 2007
Thanks much for looking at this for us! Someone will
definitely fold in the patch to initialize; we'll have to
think a bit about how the other fix will fold into the
protocol description language, I think.
Bart
In message <46D30E40.8010902 at codethink.co.uk> you wrote:
> Hi, I've been attempting to port some XSync extension code to xcb
> (libidletime in OHM [1]), and I've found a few pretty large issues.
>
> First is easy to fix- Initialize was incorrectly defined, patch attached.
>
> The second issue is a bit more systemic. The struct
> xcb_sync_systemcounter_t is generated correctly, and on the wire the
> name starts at a 14 byte offset. However thanks to compiler struct
> padding, sizeof (xcb_sync_systemcounter_t) is 16.
> This results in iteration of xcb_sync_systemcounter_iterator_t going
> wrong (by two bytes every time), and xcb_sync_systemcounter_name
> returning a pointer thats two bytes too far on.
>
> In the old Xlib code, they got around this issue by using
>
> #if ((defined(__STDC__) || defined(__cplusplus) || defined(c_plusplus))
> && !defined(UNIXCPP)) || defined(ANSICPP)
> #define _SIZEOF(x) sz_##x
> #define SIZEOF(x) _SIZEOF(x)
> #else
> #define SIZEOF(x) sz_/**/x
> #endif /* if ANSI C compiler else not */
>
> defining sz_StructName for all structures with the actual wire size, and
> using the SIZEOF macro rather than relying on the compilers idea of sizeof.
>
> Do you guys think this would be the right way to go? Another option
> might be to play around with gccs __attribute__, but that'd be pretty
> problematic for portability.
>
> I've attached a testcase for looking at these issues. Also attached is a
> couple of helper functions for helping deal with xcb_sync_int64_t, I
> guess this should probably go in xcb/util somewhere.
>
> Thanks,
> Rob Taylor
>
> [1] http://ohm.freedesktop.org
>
>
> --
> Rob Taylor, Codethink Ltd. - http://codethink.co.uk
>
> --------------060806010903020901070508
> Content-Type: text/x-patch;
> name="0001-fix-XSync-Initialize-call.patch"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline;
> filename="0001-fix-XSync-Initialize-call.patch"
>
> From 5ed49c7a3bc8fa4694884b609aba49e8f4c043cd Mon Sep 17 00:00:00 2001
> From: Rob Taylor <rob.taylor at codethink.co.uk>
> Date: Mon, 27 Aug 2007 14:56:45 +0100
> Subject: [PATCH] fix XSync Initialize call
>
> Initialize takes two CARD8 fields for desired Xsync version.
> ---
> src/sync.xml | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/src/sync.xml b/src/sync.xml
> index 74281f1..fc3c78c 100644
> --- a/src/sync.xml
> +++ b/src/sync.xml
> @@ -78,10 +78,14 @@ for licensing information.
> </error>
>
> <request name="Initialize" opcode="0">
> + <field type="CARD8" name="desired_major_version" />
> + <field type="CARD8" name="desired_minor_version" />
> + <pad bytes="2" />
> <reply>
> <pad bytes="1" />
> <field type="CARD8" name="major_version" />
> <field type="CARD8" name="minor_version" />
> + <pad bytes="22" />
> </reply>
> </request>
>
> --
> 1.5.3.GIT
>
>
>
> --------------060806010903020901070508
> Content-Type: text/x-csrc;
> name="test-xsync.c"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline;
> filename="test-xsync.c"
>
> #include <stdlib.h>
> #include <stdio.h>
> #include <xcb/xcb.h>
> #include <xcb/sync.h>
> #include "xcb_sync_aux.h"
>
> int main()
> {
> xcb_connection_t *connection;
> const xcb_query_extension_reply_t *xsync_ext;
> xcb_generic_error_t *err;
> xcb_sync_initialize_reply_t *init_reply;
> xcb_sync_list_system_counters_reply_t *xsync_list_r;
> xcb_sync_systemcounter_iterator_t i;
>
> xcb_sync_counter_t counter_id;
>
> connection = xcb_connect (NULL, NULL);
>
> /* bugger */
> if (xcb_connection_has_error(connection)) {
> printf ("no X connection!\n");
> return 1;
> }
>
> printf("connected\n");
>
> xsync_ext = xcb_get_extension_data (connection, &xcb_sync_id);
>
> if (!xsync_ext) {
> printf("no Xsync extension!\n");
> return 1;
> }
>
> printf ("first_event=%d, first_error=%d\n", xsync_ext->first_event, xsync_ext->first_error);
>
> init_reply = xcb_sync_initialize_reply (connection, xcb_sync_initialize (connection, 0, 0), &err);
>
> if (!init_reply) {
> printf ("XSync initialize failed: %d %d\n", err->response_type, err->error_code);
> return 1;
> }
>
> /* Test system counters */
> xsync_list_r = xcb_sync_list_system_counters_reply(connection, xcb_sync_list_system_counters(connection), NULL);
> i = xcb_sync_list_system_counters_counters_iterator (xsync_list_r);
> for (; i.rem ; xcb_sync_systemcounter_next (&i)) {
> printf ("SystemCounter %s found.\n", xcb_sync_systemcounter_name(i.data));
> }
>
> /* create a counter */
> counter_id = xcb_generate_id (connection);
> xcb_sync_create_counter (connection, counter_id, int64_to_xcb_sync_int64(0x2000000400000ll));
>
>
> {
> xcb_sync_query_counter_reply_t *qcr;
>
> qcr = xcb_sync_query_counter_reply (connection, xcb_sync_query_counter (connection, counter_id), &err);
> if (!qcr) {
> printf ("xcb_sync_query_counter failed: %d %d\n", err->response_type, err->error_code);
> return 1;
> }
> printf ("counter value = 0x%016llx\n", xcb_sync_int64_to_int64(qcr->counter_value));
> if (xcb_sync_int64_to_int64 (qcr->counter_value) !=0x2000000400000ll) {
> printf ("Counter value incorrect!\n");
> return 1;
> }
> }
>
> return 0;
> }
>
>
> --------------060806010903020901070508
> Content-Type: text/x-chdr;
> name="xcb_sync_aux.h"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline;
> filename="xcb_sync_aux.h"
>
> #ifndef __XCB_SYNC_AUX_H__
> #define __XCB_SYNC_AUX_H__
>
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
>
> inline xcb_sync_int64_t
> int64_to_xcb_sync_int64 (int64_t i)
> {
> xcb_sync_int64_t n;
> n.lo = i;
> n.hi = (uint32_t) (((uint64_t) i) >> 32);
> return n;
> }
>
> inline int64_t
> xcb_sync_int64_to_int64 (xcb_sync_int64_t n)
> {
> return (int64_t)((((uint64_t)n.hi) << 32) | ((uint64_t)n.lo));
> }
>
>
> #ifdef __cplusplus
> }
> #endif
>
>
> #endif /* __XCB_SYNC_AUX_H__ */
>
>
> --------------060806010903020901070508
> Content-Type: text/plain; charset="us-ascii"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline
>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
> --------------060806010903020901070508--
More information about the Xcb
mailing list