[Xcb] EWMH API for XCB
arnaud at andesi.org
Tue Dec 15 16:14:47 PST 2009
>>>>> Jamey Sharp <jamey at minilop.net> writes:
> Yeah, the data_len parameter in xcb_ewmh_send_client_message isn't
> a constant, so it can't be checked at compile time. I meant that
> you'd add something like this in every caller:
> BUILD_BUG_ON(sizeof data > sizeof ((xcb_client_message_event_t *)
> Unlike an assertion, this doesn't generate any code; instead the
> compiler can prove that all those calls are safe, but only if
> you've checked all the callers, which is why I suggested making
> this function static. Also, you could wrap
> xcb_ewmh_send_client_message up in a macro that does this, if you
>> I'm a bit confused by this macro and does not understand anything
>> besides of the basic principle. I was thinking about simply
>> adding an assertion otherwise.
> Absolutely, that would be better than nothing.
Well, I think it would be better to not make it static as it could be
useful to any program implementing extensions to EWMH. Therefore, I have
just added an assert(). I think it's ok this way, isn't it?
> I'm not talking about different connections. I'm talking about
> different screens on the same connection. All the atoms are
> guaranteed to be the same in that case, except for _NET_WM_CM_Sn.
Oh ok, I didn't think at all about that as I have never used such
configuration. Therefore, I removed root member of xcb_ewmh_connection_t
in favor of screens and nb_screens. I added a parameter, namely the
screen number, to all the functions using the root window. Also, I
modified _NET_WM_CM_Sn which is now per-screen (xcb_atom_t *). I have
committed it there.
> The patch looks good to me, except that I think it's wrong. :-)
> You're assigning an XID to a dereferenced pointer of type "char",
> as far as I can tell. I think you need one more cast on the
> left-hand side, something like this:
> *(xcb_atom_t *) ((char *) ewmh + ewmh_atoms[i].m_offset) =
Hrm ok, oops... I have silently and discretly edited my previous commit
to hide that :).
>> That's why I have only defined accessors for
> Yes, all your functions are named *_request_counter, but they all
ewmh-> _NET_WM_SYNC_REQUEST. Isn't that wrong?
> And the comment header still has that typo of missing a "T". ;-)
I did not see it at all. Thanks much! I have just committed it.
>> > Do you really want it in the xcb-util bundle though?
>> Well, I think it's better to keep it within xcb-util.
> I'd be interested in hearing your thoughts on the discussion
> Julien and I had today on this topic.
> I think the history of free software contradicts this. For
> example, bundling lots of code into libX11 certainly didn't help
> it be maintained any better. :-) Conversely, lots of projects
> abandoned by their original authors have been picked up by
> somebody else who needed them, despite not being bundled with
> anything else.
>> Moreover, xcb-util/icccm and xcb-util/image are already used in
>> other projects, but are still parts of xcb-util.
> Yes, so are aux, and render-util I think, and awesome is using the
> property and event libraries apparently. And yes, I'd like all of
> those split out, but just because that hasn't happened doesn't
> mean we should add anything more to the mess.
I think it does make sense and I will put it in a separate repository
then. However, I think it would be good to share at least configure.ac,
coding style (I have not found anything like that except by looking at
existing code but it's pretty unconsistent ATM AFAIK), and Doxygen
configuration in order to have something consistent across libraries,
even though I'm not sure how it could be achieve. Maybe a document on
the wiki giving some templates such as Doxygen and configure.ac and
another one for the coding style as it's more general?
Thank you very much for all your comments. The code looks much better
More information about the Xcb