All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.vnet.ibm.com>
To: Jianjun Duan <duanj@linux.vnet.ibm.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
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
Subject: Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v9 2/3] migration: migrate QTAILQ
Date: Mon, 31 Oct 2016 19:01:26 +0100	[thread overview]
Message-ID: <8565b420-bb18-0907-8433-9f66285f7d80@linux.vnet.ibm.com> (raw)
In-Reply-To: <e50b71d3-e820-1243-1f21-b9b6cdc130b3@linux.vnet.ibm.com>



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

  reply	other threads:[~2016-10-31 18:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-27 22:28 [Qemu-devel] [QEMU PATCH v9 0/3] migration: migrate QTAILQ Jianjun Duan
2016-10-27 22:28 ` [Qemu-devel] [QEMU PATCH v9 1/3] migration: extend VMStateInfo Jianjun Duan
2016-10-27 22:28 ` [Qemu-devel] [QEMU PATCH v9 2/3] migration: migrate QTAILQ Jianjun Duan
2016-10-28 19:06   ` Dr. David Alan Gilbert
2016-10-28 19:46     ` Jianjun Duan
2016-10-31 13:10       ` Halil Pasic
2016-10-31 17:10         ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-31 17:21           ` Halil Pasic
2016-10-31 17:32             ` Jianjun Duan
2016-10-31 18:01               ` Halil Pasic [this message]
2016-10-31 18:25                 ` Jianjun Duan
2016-10-31 19:02                   ` Halil Pasic
2016-11-01 15:02         ` [Qemu-devel] " Paolo Bonzini
2016-11-01 15:47           ` Halil Pasic
2016-10-27 22:28 ` [Qemu-devel] [QEMU PATCH v9 3/3] tests/migration: Add test for QTAILQ migration Jianjun Duan
2016-10-28 13:57 ` [Qemu-devel] [QEMU PATCH v9 0/3] migration: migrate QTAILQ no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8565b420-bb18-0907-8433-9f66285f7d80@linux.vnet.ibm.com \
    --to=pasic@linux.vnet.ibm.com \
    --cc=amit.shah@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=blauwirbel@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=dgilbert@redhat.com \
    --cc=dmitry@daynix.com \
    --cc=duanj@linux.vnet.ibm.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=leon.alrae@imgtec.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rth@twiddle.net \
    --cc=veroniabahaa@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.