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? Halil
