Make safe double remove?

[Jonathan Howard]

More of a for argument as I see it. With unlinked state you can have if 
desired a wl_list_unlinked function (identical to empty as used on the 
head.)

Changing wl_list_insert is a code breaker as it is used to initialise at 
the moment so can't inspect the element. There remains other unsafe 
calls you should not do.

wl_list_remove changed isn't 100% guaranteed to not break anything, on 
the other hand there is no guarantee the code does not have the fatal 
double remove already. I would rather not have to debug to figure out 
remove was double called as the extremely likely fix is correcting that 
I wanted the call to be safe.

On 28/01/14 19:36, Jason Ekstrand wrote:
> Jonathan,
> I have thought on multiple occations about doing this exact thing.  If
> we do, we should probably also make wl_list_insert_list safe.
>
> The counter-argument for all this is that you have to keep track of
> whether or not your elements are in a list anyway to avoid corruption.
> For instance, you still have to watch for double wl_list_insert which
> will also break things badly.  You could say, "We'll modify
> wl_list_insert so it's safe too" but then you inserting an element that
> is already in a list becomes "safe" from a segfault perspective and you
> make it easy to have program logic errors that don't result in a crash,
> just things being subtly wrong and impossible to track down.
>
> All said, I think I'd be in favor of making double-remove safe.
> However, it's not a silver bullet.
> --Jason Ekstrand
>
> On Jan 28, 2014 11:22 AM, "Jonathan Howard" <jonathan at unbiased.name
> <mailto:jonathan at unbiased.name>> wrote:
>
>     Code is already using wl_list_init to have the construction of
>     unlinked items in a stable state so that they can be destroyed with
>     wl_list_remove.
>
>     A problem is it is easy to write code that calls wl_list_remove
>     during the structures life and miss that you need to reset with
>     wl_list_init for when it is later destroyed.
>
>     Having wl_list_remove place items in this unlinked state makes the
>     destruction safe (and makes quite a few lines of code redundant.)
>
>     --- wayland-util.c      2014-01-28 16:09:13.179052955
>     <tel:13.179052955> +0000
>     +++ wayland-util.c.init 2014-01-28 16:13:32.225704904 +0000
>     @@ -52,8 +52,8 @@
>       {
>              elm->prev->next = elm->next;
>              elm->next->prev = elm->prev;
>     -       elm->next = NULL;
>     -       elm->prev = NULL;
>     +       elm->next = elm;
>     +       elm->prev = elm;
>       }
>
>       WL_EXPORT int
>
>     _________________________________________________
>     wayland-devel mailing list
>     wayland-devel at lists.__freedesktop.org
>     <mailto:wayland-devel at lists.freedesktop.org>
>     http://lists.freedesktop.org/__mailman/listinfo/wayland-devel
>     <http://lists.freedesktop.org/mailman/listinfo/wayland-devel>
>