This issue was previously reported as
http://bugzilla.gnome.org/show_bug.cgi?id=341573 but I can't find any xorg bug
that was filed.
The XSizeHints and XWMHints structs are both documented as
/∗ this structure may be extended in the future */
and for both there are allocation functions provided (XAllocWMHints and
XAllocSizeHints), presumably so that callers to XGetWMHints, XGetWMNormalHints,
and XGetWMSizeHints can have the writer of the struct allocate it. The
writer-allocates contract provides binary compatibility both for an application
running against a newer (members added) library than the one it was compiled
against and for an application running against an older library, and is the only
reason I can see to have an allocation function.
The corresponding setter functions (XSetWMHints, XSetWMNormalHints, and
XSetWMSizeHints, and also the old XSetSizeHints and XSetNormalHits, which live
in SetHints.c and SetNrmHint.c) currently read all the known members
unconditionally out of the provided struct. This causes valgrind warnings
("syscall param write(buf) points to uninitialised byte(s)") if the caller did
not initialize all the members -- something that the extensibility of the
structure means will certainly happen when the structure is extended. (These
warnings are reported in http://bugzilla.gnome.org/show_bug.cgi?id=341573 and I
also see them starting up Mozilla Firefox.)
Given the extensibility contract, it seems reasonable for callers to assume that
they do not need to initialize members if they don't set the bitfield member
saying that they are setting those members. The GDK developers (rightly, in my
opinion) marked http://bugzilla.gnome.org/show_bug.cgi?id=341573 as not being
Reading data that the client does not indicate is set could even cause a
segmentation fault if the struct is extended and the caller has only allocated
space for the old, smaller, struct.
I think the five functions mentioned:
XSetWMHints, XSetWMNormalHints, XSetWMSizeHints, XSetSizeHints and
and perhaps others that I don't know about should check the flags member of the
struct they are given and only read members that correspond to the flags that
are set: they should instead use zero in place of the members not set according
to the flags member.
Created attachment 8435 [details] [review]
I tested that this patch fixes the valgrind warnings when starting Mozilla in
libX11 pulled from git this morning, built with --without-xcb (since xsltproc
wouldn't behave during xcb compilation).
I'm not sure that (USPosition|PPosition) and (USSize|PSize) are the right
things to test in XSetSizeHints and XSetWMSizeHints (marked with XXX comments).
Sorry about the phenomenal bug spam, guys. Adding xorg-team@ to the QA contact so bugs don't get lost in future.
In XSetWMSizeHints(), the memset() needs to be moved up by one line -- otherwise you end up with data.flags being zero all the time.
Created attachment 9361 [details] [review]
Oops. Corrected per comment 3.
I committed your patch in 0284b144340a455a4b5b5011d81ac5a610372291. I think the code marked with XXX is correct, so I removed those markers.