From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60540) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJidx-0002nx-Fa for qemu-devel@nongnu.org; Mon, 27 Jul 2015 09:40:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZJidw-0000ru-1l for qemu-devel@nongnu.org; Mon, 27 Jul 2015 09:40:37 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:100::1]:43538) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZJidv-0000rk-FI for qemu-devel@nongnu.org; Mon, 27 Jul 2015 09:40:35 -0400 Date: Mon, 27 Jul 2015 15:40:32 +0200 From: Aurelien Jarno Message-ID: <20150727134032.GN11361@aurel32.net> References: <1437994568-7825-1-git-send-email-aurelien@aurel32.net> <1437994568-7825-6-git-send-email-aurelien@aurel32.net> <55B61562.2030808@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55B61562.2030808@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 for-2.5 05/12] tcg/optimize: track const/copy status separately List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Richard Henderson 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