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

* "Thomas Leonard" <[email protected]> [2008-05-25 12:39]:
>
> 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:  

Which application? 32 or 64 bit system?

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

I'll check (hopefully) tomorrow on a 64 bit system at work and come
with a fixed version of my patch.

> > +       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?  

Negative means very large in case you read as 'uint'. So that's not a
problem here.

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

True. Expect I should have tried that 'fixed' patch before. :(



	Bernhard

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