From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42175) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1Gu0-0007nB-SV for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:01:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1Gtx-00025G-PE for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:01:44 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:36743 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1Gtx-000257-Jp for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:01:41 -0400 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9VHt3wu142084 for ; Mon, 31 Oct 2016 14:01:40 -0400 Received: from e06smtp08.uk.ibm.com (e06smtp08.uk.ibm.com [195.75.94.104]) by mx0b-001b2d01.pphosted.com with ESMTP id 26eab4atyc-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 31 Oct 2016 14:01:40 -0400 Received: from localhost by e06smtp08.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 31 Oct 2016 18:01:36 -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> From: Halil Pasic Date: Mon, 31 Oct 2016 19:01:26 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Message-Id: <8565b420-bb18-0907-8433-9f66285f7d80@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v9 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 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, pbonzini@redhat.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au On 10/31/2016 06:32 PM, Jianjun Duan wrote: >>>> I think this got overly complicated. 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. >>> >> It is unlikely the definition of QTAILQ will change, so hard-coded >>> >> value probably was the most simple. Now that we want to address the >>> >> potential changes, I think my code will deal with future changes better. >> > >> > Please elaborate in what way does your version deal better with future >> > changes? IMHO the version with my patch applied covers all the corners >> > your original code covers but it is without any doubt more concise and >> > in my opinion also more straight-forward. > I don't use the internals of head and entry structures if there are > access macro around. Also I didn't use pointer type cast. I don't think > pointer cast is more straightforward. Internals is quite relative since the stuff is part of the header and there is no indication that it's not part of the API. About pointer type casts I do not understand what you mean by that: my understanding is that we both do casts to a pointer type. And I do think it is more straightforward than defining a macro for the offset for each link-pointer and then basically play the field_at_offset game twice: once to pin point the struct holding all the links, and once again to pinpoint the individual links. As you see I do not believe you that your version is more robust. If you want to convince me _give me a remotely reasonable patch_ which beaks my version but not yours. The straight-forwardness is probably a matter of taste, and that's why I used IMHO. I'm not sure if I understood you correctly, do you think that the current version of the code is more readable that what I have proposed? If you do, I doubt there is a rational way to settle this taste dispute. If the majority of the people says your approach is more straight-forward then I apologize. Cheers, Halil