On Tue, 20 Oct 2020 at 12:17, Peng Liang <liangpen...@huawei.com> 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é <phi...@redhat.com> > > > > 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
