Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v9 2/3] migration: migrate QTAILQ

[Jianjun Duan]

On 10/31/2016 12:02 PM, Halil Pasic wrote:
> 
> 
> On 10/31/2016 07:25 PM, Jianjun Duan wrote:
>>
>>
>> On 10/31/2016 11:01 AM, Halil Pasic wrote:
>>>
>>>
>>> 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.
>>>
>> It is more concise but not more straightforward for me.
>>
>>> 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.
>>>
>>
>> I agree it is a matter of taste. With current code both versions will
>> not break. But if somebody changes the name of the var you directly
>> accessed, your code will break. Of course I don't expect somebody to
>> just do that.
>>
>> It is indeed in the header file and we expect people to change all at
>> once if any change is done. I used this argument for hard encoding and
>> it is not taken.
> 
> There is a huge difference. If somebody changes the name of tqe_next
> for instance it will break compile-time and it will also break the
> normal (non-raw) macros. Same goes for the case if someone wanted to
> remove the Q_TAILQ_HEAD macro for example (or am I missing something). 
> In your case there was no compile time error but a possible run-time
> memory corruption instead. If you see no difference between these I
> really do not know how to do constructive discussion.
> 
Are you suggesting that all the macros used to access data elements
should be avoided? What you suggested is the inherent issues with macros.

It is more likely the internal of a structure be changed than the access
interfaces be changed.


Thanks,
Jianjun
> Cheers,
> Halil
> 
>>
>> Thanks,
>> Jianjun
>>  > Cheers,
>>> Halil
>>>
> 
>