[PATCH 2/4] Free the data returned by XGetWindowProperty()

[Eirik Byrkjeflot Anonsen]

Alan Coopersmith <alan.coopersmith at oracle.com> writes:

> On 05/20/15 05:58 AM, Eirik Byrkjeflot Anonsen wrote:
>> Eirik Byrkjeflot Anonsen <eirik at eirikba.org> writes:
>>
>>> Signed-off-by: Eirik Byrkjeflot Anonsen <eirik at eirikba.org>
>>> ---
>>>   xprop.c | 7 +++++--
>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xprop.c b/xprop.c
>>> index ba23b87..010eff4 100644
>>> --- a/xprop.c
>>> +++ b/xprop.c
>>> @@ -1442,9 +1442,12 @@ Get_Window_Property_Data_And_Type (Atom atom,
>>>       unsigned long nitems;
>>>       unsigned long nbytes;
>>>       unsigned long bytes_after;
>>> -    unsigned char *prop;
>>> +    static unsigned char *prop = NULL;
>>>       int status;
>>> -	
>>> +
>>> +    if (prop)
>>> +	XFree(prop);
>>> +
>>>       status = XGetWindowProperty(dpy, target_win, atom, 0, (max_len+3)/4,
>>>   				False, AnyPropertyType, &actual_type,
>>>   				&actual_format, &nitems, &bytes_after,
>>
>> Though now that I look at this one, I wonder whether I can really trust
>> XGetWindowProperty() to always set 'prop', even if it fails. Maybe I
>> should add "prop = NULL" after the call to XFree()?
>
> You can't, since we didn't fix Xlib to always set it until just recently:
> http://cgit.freedesktop.org/xorg/lib/libX11/commit/src/GetProp.c?id=192bbb9e2fc45df4e17b35b6d14ea0eb418dbd39

Nice :)

> In this case, you'd be safe as long as no one removes the Fatal_Error
> calls on failure, but because we can't guarantee that will never happen,
> I'd be happier with setting it to NULL after the XFree, since that's
> cheap and lowest risk.

Agreed. I'll make a v2.

eirik