Ok, so I applied the attached patch for debugging, and here is the output: # move mouse IN flower XXX surface NULL # move mouse OUT of flower XXX Prevented BAD THINGS FROM HAPPENING # move mouse IN flower again XXX surface NULL # move mouse OUT of flower XXX Prevented BAD THINGS FROM HAPPENING # move mouse IN flower again (etc ...) YYY surface NULL # move mouse out of wayland (it is running in X, so just move mouse outside of wayland window) YYY Prevented BAD THINGS FROM HAPPENING # move mouse IN wayland window
So I thing this clarifies and proves to some extend my explanation in the previous email ;-) Regards, Iskren On Sat, Mar 12, 2011 at 10:54 PM, Iskren Chernev <iskren.cher...@gmail.com>wrote: > In my opinion the problem is this: > The code to remove the element from the list is called every time, and the > code to insert it is called only when 'surface'. So in case surface is NULL > the item won't be added, but will be removed next time the function is > entered. Isn't that the case? > > I don't have a detailed understanding of this exact code, but isn't the > explanation as simple as that? > Should I add some debugging info to see if this is indeed the case, or you > are sure that the problem lies somewhere else? > > About the 'tagging' -- are you proposing to add a boolean flag to the > listener to explicitly mark it as 'in a list' or 'out of a list'? > > Regards, > Iskren > > On Sat, Mar 12, 2011 at 9:48 PM, Marty Jack <marty...@comcast.net> wrote: > >> >> >> On 03/12/2011 02:20 PM, Bill Spitzak wrote: >> > On 03/12/2011 04:28 AM, Marty Jack wrote: >> >> I have never encountered a system where it was believed to be desirable >> to allow something to be removed twice. It is important to keep data >> structures clean. If anything you would be more likely to see a debugging >> mode where the lists were fully checked after every insert or remove to make >> sure they are internally consistent, especially if they are important to >> keeping the system running. It's not that much different from memory >> allocation. A block is allocated, or it is free, and a double free is a >> bug. >> > >> > Making it crash at the moment the second remove is attempted is better >> than it leaving the data corrupted and crashing later. It makes it a lot >> easier to find out why it went wrong. >> > >> > I think that is what the newest version of the patch is doing, right? >> > _______________________________________________ >> > wayland-devel mailing list >> > wayland-devel@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> > >> >> Oh and no, the second version of the patch makes the second remove a >> no-op. >> _______________________________________________ >> wayland-devel mailing list >> wayland-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> > >
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c index 74e5f01..12ce8ae 100644 --- a/wayland/wayland-server.c +++ b/wayland/wayland-server.c @@ -371,9 +371,13 @@ wl_input_device_set_pointer_focus(struct wl_input_device *device, if (wl_list_in(&device->pointer_focus_listener.link)) wl_list_remove(&device->pointer_focus_listener.link); + else + fprintf(stderr, "XXX Prevented BAD THINGS FROM HAPPENING\n"); if (surface) wl_list_insert(surface->destroy_listener_list.prev, &device->pointer_focus_listener.link); + else + fprintf(stderr, "XXX surface NULL\n"); } WL_EXPORT void @@ -402,9 +406,13 @@ wl_input_device_set_keyboard_focus(struct wl_input_device *device, if (wl_list_in(&device->keyboard_focus_listener.link)) wl_list_remove(&device->keyboard_focus_listener.link); + else + fprintf(stderr, "YYY Prevented BAD THINGS FROM HAPPENING\n"); if (surface) wl_list_insert(surface->destroy_listener_list.prev, &device->keyboard_focus_listener.link); + else + fprintf(stderr, "YYY surface NULL\n"); } WL_EXPORT void
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
