Philippe Mathieu-Daudé <phi...@linaro.org> writes:
> (Cc'ing QAPI maintainer)
>
> On 24/11/23 02:53, Daniel Hoffman wrote:
>> This was the only failure preventing `make check` from passing with
>> sanitizers
>> enabled on my configuration.
>
> IIUC this is due to visit_start_list() which expects a NULL list,
> see qapi/qapi-visit-core.c:
>
> bool visit_start_list(Visitor *v, const char *name, GenericList **list,
> size_t size, Error **errp)
> {
> bool ok;
>
> assert(!list || size >= sizeof(GenericList));
This asserts either "if real walk, then size is sane".
>
> which is well defined in its declaration:
>
> /*
> * Start visiting a list.
> *
> * @name expresses the relationship of this list to its parent
> * container; see the general description of @name above.
> *
> * @list must be non-NULL for a real walk, in which case @size
> * determines how much memory an input or clone visitor will allocate
> * into *@list (at least sizeof(GenericList)). Some visitors also
> * allow @list to be NULL for a virtual walk, in which case @size is
> * ignored.
> ...
Mind the number of *!
The function contract talks about @list, which is GenericList **.
get_prop_array() passes &list, where list is GenericList *. &list is
non-null even before the patch.
The patch initializes @list to empty.
> With the patch description improved:
Specifically, show how things fail under sanitation.
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>
>> Signed-off-by: Daniel Hoffman <dhoff...@gmail.com>
>> ---
>> hw/core/qdev-properties.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 91632f7be9f..4caa78b7bc5 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -690,7 +690,7 @@ static void get_prop_array(Object *obj, Visitor *v,
>> const char *name,
>> uint32_t *alenptr = object_field_prop_ptr(obj, prop);
>> void **arrayptr = (void *)obj + prop->arrayoffset;
>> char *elem = *arrayptr;
>> - GenericList *list;
>> + GenericList *list = NULL;
>> const size_t list_elem_size = sizeof(*list) + prop->arrayfieldsize;
>> int i;
>> bool ok;
if (!visit_start_list(v, name, &list, list_elem_size, errp)) {
return;
}
