[Xcb] bug in icccm.c ?

[Jamey Sharp]

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.

> 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.

--Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/xcb/attachments/20060905/c3ca573d/attachment.pgp