[Xcb] bug in icccm.c ?

Vincent Torri vtorri at univ-evry.fr
Wed Sep 6 00:56:12 PDT 2006

On Tue, 5 Sep 2006, Jamey Sharp wrote:

> On Tue, Sep 05, 2006 at 10:01:05AM +0200, Vincent Torri wrote:
>> (of course, that remarks also applies with props.c, and maybe other
>> functions in xcb-utils)
> First disclaimer: the original libraries in xcb-util are still lacking a
> coherent design, and may be full of bugs. icccm is one of those
> libraries; property is another. They shouldn't be used in real projects
> without some discussion on the list here about their many flaws. :-) I
> don't know of anything but my window manager demo that uses them, and it
> doesn't adequately test them.

Right, they need a complete review. The problem is that I have to use them 
in ecore. It's the fourth time I rewrite ecore_xcb (used later for 
enlightenment). I want it to be the last (or at least with small 
modifications later, and not in the design). Ecore will test them all :) 
It will be a good test.

I also think that making unit test is recessary for icccm.

>> I've noticed that all the SetFlag* functions set the flag, but do no add
>> it :
>> hints->flags = ***
>> and not
>> hints->flags |= ***
>> Is it normal ?
> I have no idea what those were for, though it looks like I'm the one who
> wrote them. Looking at them now, they seem useless to me. The functions
> that follow -- SizeHintsSetPosition, etc. -- do things that I think are
> sensible though.
>> Also, I would like to add functions similar to those in icccm.c, but which
>> take a reply, so that we can avoid round trips. Currently, we just have
>> mimic the Xlib functions, hence, we have suppressed all the benefit of
>> xcb. What do you think ?
> According to git history, you wrote GetWMSizeHints, so I guess you're
> welcome to change how it works. :-)


> By the way, on inspection, GetWMSizeHints has at least two correctness
> bugs and one memory leak that mean it couldn't have worked anyway. At
> first I thought I wrote it, bugs and all -- it mostly looks like my
> style, and does a number of good things, like handling pre-ICCCM
> clients. Anyway, since you want to redesign it, a rewrite might be in
> order rather than fixing the bugs.
> One approach is to mimic xcb_atom's InternAtomFast function, which
> returns a tagged union containing either the actual atom or an
> XCBInternAtom cookie. When you need the value later, the
> InternAtomFastReply function updates the tagged union in-place with the
> actual value, if it wasn't already there. All of the SizeHintsGet*
> functions could work like InternAtomFastReply.
> Another approach is to provide simple wrappers for XCBGetProperty and
> XCBGetPropertyReply that just provide the right parameters and unpack
> the results correctly. That may be better in this case.

I prefer the second one. Maybe because the first one can later use the 
second one :)

I would write 3 functions : wrappers for XCBGetProperty and 
XCBGetPropertyReply, and another one that uses the reply only, as it can 
be used for other functions.

I also think that I have to wrap the ***Uncheck function from plan 7.

Also, we have already mention that before in the mailing list. If I 
rewrite the functions, which naming convention should I use ? I 
really don't like the Xlib one.


More information about the Xcb mailing list