Re: [Qemu-devel] [PATCH v2 for-2.5 05/12] tcg/optimize: track const/copy status separately

[Aurelien Jarno <aurelien@aurel32.net>]

On 2015-07-27 13:26, Paolo Bonzini wrote:
> 
> 
> On 27/07/2015 12:56, Aurelien Jarno wrote:
> >          temps[dst].next_copy = temps[src].next_copy;
> >          temps[dst].prev_copy = src;
> >          temps[temps[dst].next_copy].prev_copy = dst;
> >          temps[src].next_copy = dst;

Note that the patch doesn't change this part, it's already there in the
original code.

> This is:
> 
>     dst->next = src->next;
>     dst->prev = src;
>     dst->next->prev = dst;
>     src->next = dst;
> 
> which seems weird.  I think it should be one of
> 
>     /* splice src after dst */
>     dst->next->prev = src->prev;
>     src->prev->next = dst->next;
>     dst->next = src;
>     src->prev = dst;
> 
> or
> 
>     /* splice src before dst */
>     last = src->prev;
>     dst->prev->next = src;
>     src->prev = dst->prev;
>     last->next = dst;
>     dst->prev = last;
> 
> (written as pointer manipulations for clarity).
> 
> Maybe I'm missing the obvious, but if it's a problem it's better to fix
> it before the links are used more pervasively.

I think your code is the generic for inserting a circular linked list
into another. Here we just want to insert one element, and thus before
the insertion we have dst->next = dst->prev = dst.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net