From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55531) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1bHY-0003B6-1Q for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:47:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1bHU-0002Bt-VR for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:47:24 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:36466) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1bHU-0002BS-Ns for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:47:20 -0400 Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uA1Fi5N4142056 for ; Tue, 1 Nov 2016 11:47:19 -0400 Received: from e06smtp11.uk.ibm.com (e06smtp11.uk.ibm.com [195.75.94.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 26es8yp32e-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 01 Nov 2016 11:47:19 -0400 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 1 Nov 2016 15:47:16 -0000 References: <1477607317-27817-1-git-send-email-duanj@linux.vnet.ibm.com> <1477607317-27817-3-git-send-email-duanj@linux.vnet.ibm.com> <20161028190657.GB2173@work-vm> <0cd200a6-3ded-0e02-1b8b-fbbf5c0bee03@linux.vnet.ibm.com> <88bb646d-39aa-e439-4b30-2c38777a4b56@linux.vnet.ibm.com> <56fd391c-0723-6bfa-e0fa-ac68af8119c9@redhat.com> From: Halil Pasic Date: Tue, 1 Nov 2016 16:47:11 +0100 MIME-Version: 1.0 In-Reply-To: <56fd391c-0723-6bfa-e0fa-ac68af8119c9@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Message-Id: <4beb0f71-514e-f7a8-5bd9-de1ede47f2bf@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v9 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Jianjun Duan , "Dr. David Alan Gilbert" Cc: veroniabahaa@gmail.com, peter.maydell@linaro.org, mdroth@linux.vnet.ibm.com, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au On 11/01/2016 04:02 PM, Paolo Bonzini wrote: > > > On 31/10/2016 14:10, Halil Pasic wrote: >> I think this got overly complicated. > > I agree. :) > >> Here is a little patch on >> top of your stuff which gets rid of 15 lines and IMHO simplifies >> things quite a bit. What do you think? >> >> It is based on/inspired by Dave's proposal with the dummy stuff. >> >> Did not address the typos though. > > I still prefer my field_at_offset proposal, but I'm obviously biased. :) have searched it *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL; *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) = *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET); **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm); *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET); That was for the insert of course. A lot of * for my taste and we still have the QTAILQ_NEXT_OFFSET and QTAILQ_PREV_OFFSET macros which are supposed to capture what QTAILQRawEntry does (namely where is the next and the prev pointer within the entry -- not the element). Obviously I'm biased too ;) > Squashing your changes on top of 2/3 is fine too. But this change makes > little sense: > >> */ >> #define QTAILQ_RAW_NEXT(elm, entry) \ >> - (*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **)) >> + ((field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_next)) >> #define QTAILQ_RAW_PREV(elm, entry) \ >> - (*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***)) >> + (field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_prev) >> + > > field_at_offset seems to be out of place. Me scratching head. This is the place where we pinpoint the qtailq entry which is at offset entry within the element and reinterpret the pointer as a pointer to QTAILQRawEntry. In the end we end up with a void pointer to the next or prev element. I think #define QTAILQ_RAW_NEXT(elm, entry_offset) \ ((field_at_offset(elm, entry_offset, QTAILQRawEntry *)->tqe_next) would be more readable though. Could you explain whats wrong with that? Thank you very much for commenting! Halil > > Paolo >