Re: [PATCH] Implement _NET_WM_ICON support for retrieving the application icon

2008/5/25 Bernhard Walle <[email protected]>:
> * "Thomas Leonard" <[email protected]> [2008-05-24 19:52]:
>>
>> Might be worth checking that width and height are sensible here
>> (within length), or a buggy app could probably crash us here...
>
> Thanks for the review. What about that updated version of the patch?

> +       if (XGetWindowProperty(xdisplay, w, gdk_x11_atom_to_xatom(xa__NET_WM_ICON), 0, G_MAXLONG,
> +                       FALSE, AnyPropertyType,  &type, &format, &length,
> +                       &rest, (guchar **)&data) != Success || length <= 2)
> +               return NULL;
> +
> +       gint width = data[0];
> +       gint height = data[1];

This doesn't work for me. I get 48 and 0. The XGetWindowProperty has a
really stupid interface; from the man-page:

"If the returned format is 32, the returned data is represented as a
long array and should be cast to that type to obtain the elements."

Yes, it actually goes to the trouble of copying the returned array of
32 bit values into a new array of 64 bit values, which then have
to be converted back again.

> +       if ((width * height * sizeof(guint32)) > length) {
> +               fprintf(stderr, "Invalid _NET_WM_ICON data. w=%d, h=%d, len=%d\n",
> +                       width, height, length);
> +               return NULL;
> +       }

This failed on my system because height was zero. Also, what if one is negative?
Also, is multiplying by sizeof(guint32) right? I think length is the
number of items,
not the number of byes. What about the two initial words?

Thanks,


-- 
Dr Thomas Leonard	http://rox.sourceforge.net
GPG: 9242 9807 C985 3C07 44A6 8B9A AE07 8280 59A5 3CC1

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/