On 10/26/20 9:25 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> We have a useful macro for inserting at the front of any >> QAPI-generated list; move it from block.c to qapi/util.h so more >> places can use it, including one earlier place in block.c. >> >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> --- >> include/qapi/util.h | 8 ++++++++ >> block.c | 15 +++------------ >> 2 files changed, 11 insertions(+), 12 deletions(-) >> >> diff --git a/include/qapi/util.h b/include/qapi/util.h >> index 50201896c7a4..b6083055ce69 100644 >> --- a/include/qapi/util.h >> +++ b/include/qapi/util.h >> @@ -30,4 +30,12 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char >> *buf, >> >> int parse_qapi_name(const char *name, bool complete); >> >> +/* For any GenericList @list, insert @element at the front. */ >> +#define QAPI_LIST_ADD(list, element) do { \ >> + typeof(list) _tmp = g_new(typeof(*(list)), 1); \ > > g_new(typeof(*p), 1) is an rather roundabout way to say > g_malloc(sizeof(*p). Yes, it returns typeof(p) instead of void *, but > the chance of this catching mistakes here rounds to zero. > > Not this patch's problem. Ignore me :)
typeof is a gcc/clang extension; using sizeof makes the code slightly more portable (but doesn't affect qemu's usage). I'm happy to change it, although it would require more commit message finesse since that becomes more than just code motion. >> @@ -5221,22 +5222,12 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat, >> qapi_free_BlockDeviceInfoList(list); >> return NULL; >> } >> - entry = g_malloc0(sizeof(*entry)); >> - entry->value = info; >> - entry->next = list; >> - list = entry; >> + QAPI_LIST_ADD(list, info); >> } >> >> return list; >> } > > PATCH 12 has more. I wonder whether PATCH 12 should be squashed into > this one. You decide. Patch 12 has a LOT more. And we're really close to soft freeze. I kept them separate to minimize the risk of landing my QAPI changes (4/12) before 5.2 (that MUST happen, or we've locked in a problematic API with block-export-add), vs. cleanup changes that may require review from a lot more areas in the tree. Given the timeline, I prefer to keep them separate for v6. I also still wonder if it is possible to make Coccinelle identify candidates, rather than my manual audit that produced patch 12. But for v6, I _will_ update the commit message to mention that more conversions are possible, and saved for a later patch. > > Reviewed-by: Markus Armbruster <arm...@redhat.com> Thanks; I think I can keep this even for v6, since all I am changing is an enhanced commit message. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org