On 10/20/2020 7:27 PM, Peter Maydell wrote: > On Tue, 20 Oct 2020 at 12:17, Peng Liang <[email protected]> wrote: >> >> On 10/19/2020 6:35 PM, Philippe Mathieu-Daudé wrote: >>> On 10/19/20 11:34 AM, Peng Liang wrote: >>>> VMStateDescription.fields should be end with VMSTATE_END_OF_LIST(). >>>> However, microbit_i2c_vmstate doesn't follow it. Let's change it. >>> >>> It might be easy to add a Coccinelle script to avoid future errors. >>> >>> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> >>> >> >> I tried to add a Coccinelle script to add VMSTATE_END_OF_LIST() to the >> end of VMStateDescription.fields. For those who are not defined as >> compound literals, it works well. However, I cannot make it work for >> those defined as compound literals. And Julia doesn't think compound >> literals are supported currently[1]. So maybe currently it's hard to >> check the error using Coccinelle :( > > I think we could probably significantly increase the chances that > people find "missing terminator" errors in the course of normal > debugging of their device if we made the terminator be something > other than "is field->name NULL". That condition is quite likely > to be satisfied by accident shortly after the real end-of-data > (because zeroes are easy to find in memory), whereas if the condition > is "field->flags is a magic number", for instance, then the chances of > it being satisfied by accident are very low, and so a simple "loop > through the field array until we find the end" is pretty likely to > hang/crash. (If we don't already have such a loop we might need to > add one in debug mode when a vmstate is registered.) > > (This is why the REGINFO_SENTINEL used for Arm cpreg arrays is > not a simple all-zeroes value, incidentally.) > > thanks > -- PMM > . >
I found that field->flags is a bit-or field, so maybe all 0xf or other magic number is still meaningful? Can we use field->version_id or field->struct_version_id as the condition? I found they are all int type but used as non-negative, so can we use field->version_id/field->struct_version_id == magic number (for example, -1) as a sentinel? -- Thanks, Peng
