[PATCH] wayland-server: fix resource destroy
tiago.vignatti at linux.intel.com
Wed Oct 12 02:02:49 PDT 2011
On 10/11/2011 09:23 PM, Kristian Høgsberg wrote:
> On Sun, Oct 9, 2011 at 2:32 PM, Tiago Vignatti
> <tiago.vignatti at linux.intel.com> wrote:
>> On 10/07/2011 10:49 AM, Fiedler, Mathias wrote:
>>> object id was read from freed memory.
>>> In rare cases i got errors where compositor was complaining about wrong or
>>> still used object ids.
>>> diff --git a/wayland/src/wayland-server.c b/wayland/src/wayland-server.c
>>> index a6cd88d..5eb7906 100644
>>> --- a/wayland/src/wayland-server.c
>>> +++ b/wayland/src/wayland-server.c
>>> @@ -313,9 +313,9 @@ WL_EXPORT void
>>> wl_resource_destroy(struct wl_resource *resource, uint32_t time)
>>> struct wl_client *client = resource->client;
>>> + uint32_t id = resource->object.id;
>>> - wl_map_insert_at(&client->objects, resource->object.id, NULL);
>>> + wl_map_insert_at(&client->objects, id, NULL);
>> object id has to be removed from the structure, using wl_map_remove. Also,
>> the way you cooked the patch is not that legible (one could easily go and
>> revert the changes for cleaning up) and I guess we can make better a bit
> No it has to be removed by inserting a NULL pointer. Mathias' patch
> is right, but I edited the fix slightly, to just insert the NULL
> pointer before calling destroy_resource().
> The map data structure can be used in one of two modes: either you use
> it to allocate IDs in constant time by using wl_map_insert_new() and
> wl_map_remove(). wl_map_insert_new() will allocate and return a new
> ID, wl_map_remove() will put the removed entry on a free list so
> insert can find it in constant time. This is how the clients allocate
> IDs and how the server allocates global object names.
> The other way is just wl_map_insert_at(), which will insert the given
> pointer and the specified index. This is how you use it when somebody
> else allocates the IDs and you're just keeping track of the mapping.
right. A bit tricky the API, but thanks for explanation! Should we amend
it on the code?
More information about the wayland-devel