[PATCH v3 0/4] xprop: Free various allocated memory

[Eirik Byrkjeflot Anonsen]

Adam Jackson <ajax at nwnk.net> writes:

> On Thu, 2015-07-23 at 11:17 +0200, Eirik Byrkjeflot Anonsen wrote:
>
>> I'm not particularly surprised, but given that the patch is made and is
>> largely trivial it would be nice to get it into the official code. Does
>> anyone have any idea how to make that happen?
>
> Apologies for the delay here, I've been going through the patchwork
> backlog and saw this series.  1/4 and 3/4 look fine, I've got ahead and
> merged them:
>
> remote: I: patch #50243 updated using rev 4f748e3d2b1368ec0590a413ba5f7addc5e3344f.
> remote: I: patch #49787 updated using rev dee1d0c1316b1c62c6c62d6f0f4b13685e8e6630.
> remote: I: 2 patch(es) updated to state Accepted.
> To ssh://git.freedesktop.org/git/xorg/app/xprop
>    b0ae4b9..dee1d0c  master -> master
>
> The other two are a little wonky at first glance.  Is there a reason to
> use that static trick instead of just freeing at the end of the
> function?  From the very briefest of glances it doesn't look like the
> pointers leak outside their caller, but maybe I missed it.
>
> - ajax

Patch 2 is the "prop" variable in Get_Window_Property_Data_And_Type, I
think. Note that the function ends with:

    return (const char*)prop;

So I rather think it would be a bad idea to free it before the end of
the function :)

Patch 4 is the "result" variable in Format_Icons(). That function also
ends with:

   return result;

So yes, I feel pretty confident in saying that these should be kept
alive beyond the end of their respective functions.

If I remember correctly, these patches frees up memory that ends up
being returned by Get_Property_Data_And_Type(). So the nice solution
would be to free them in Show_Prop() once that function is done with it.
However, the various data that may be returned from
Get_Property_Data_And_Type() is not all allocated in the same way. If I
remember correctly there are at least some values that are malloced,
some that should be XFreed and some that are statically allocated.

An alternative solution would be to change all functions that return
data through Get_Property_Data_And_Type() to return malloced data. Then
we could add the free() in Show_Prop(). But that is rather a bigger
change.

eirik