On 10/27/2016 04:16 AM, Halil Pasic wrote: > > > On 10/26/2016 07:33 PM, Jianjun Duan wrote: >>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 >>>>>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>>>>>>>>>> >>>>>>>>>>>> OK, you might want to add some asserts somewhere in a .c just to >>>>>>>>>>>> catch if >>>>>>>>>>>> any of these offsets change. >>>>>>>>>>>> >>>>>>>>>> if the layout of QTAILQ changes, the version id for the vmsd should >>>>>>>>>> also >>>>>>>>>> be changed. It will break migration between different versions. >>>>>>>>>> However >>>>>>>>>> the operation doesn't depend on these values. >>>>>>>> >>>>>>>> No, changing layout of QTAILQ doesn't need to change the version id of >>>>>>>> vmsd; >>>>>>>> it's an internal change. But if someone does make the change and >>>>>>>> forgets >>>>>>>> to update your OFFSET macros it'll cause memory corruption. >>>>>>>> You could catch that with an assert (possibly build time). >>>>>>>> >>>>>> If we use these constant I would agree an assertion is necessary. By >>>>>> using a macro we leave the door open for change. and if someone changes >>>>>> the layout, he or she better change the macros and the version id. If an >>>>>> assertion is added, then that assertion needs to be changed to reflect >>>>>> the change, then in the unlikely situations we could have several >>>>>> version of layout/macro/assertions floating around. That is too much. SO >>>>>> version id is the best guard here. >>>> >>>> Version id's are irrelevant here. >>>> The macros are irrelevant here. >>>> I'm talking about a potential mismatch between the definition of >>>> QTAILQ_LAST_OFFSET >>>> and the definition of Q_TAILQ_HEAD. >>>> >>>> Dave >>>> >> I suppose anyone who changes the layout should also change the macro and >> version ID of the relevant vmsd. Similar issue was discussed before as >> the early version tried to generate all these offsets based on the >> element and head type. You can see in version 2 discussion. >> >> >> Thanks, >> Jianjun > > IMHO Dave is right, although a change in head/entry does not seem too > likely to me. Just want to point out that Dave's proposal (discussed here > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03770.html) > does not have these issues and it appears more elegant to me. Jianjun > why do you prefer doing the offsets manually? > It came to the current shape based on previous comments stretching back almost 6 months. I can change it to whatever reviewers want.
Thanks, Jianjun > Halil > >
