Re: [PATCH v5 02/12] qapi: Make QAPI_LIST_ADD() public

[Markus Armbruster]

Eric Blake <eblake@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 <vsementsov@virtuozzo.com>
> Signed-off-by: Eric Blake <eblake@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 :)

> +    _tmp->value = (element); \
> +    _tmp->next = (list); \
> +    (list) = _tmp; \
> +} while (0)
> +
>  #endif
> diff --git a/block.c b/block.c
> index 430edf79bb10..45bd79299611 100644
> --- a/block.c
> +++ b/block.c
> @@ -39,6 +39,7 @@
>  #include "qapi/qmp/qstring.h"
>  #include "qapi/qobject-output-visitor.h"
>  #include "qapi/qapi-visit-block-core.h"
> +#include "qapi/util.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/notify.h"
> @@ -5211,7 +5212,7 @@ BlockDriverState *bdrv_find_node(const char *node_name)
>  BlockDeviceInfoList *bdrv_named_nodes_list(bool flat,
>                                             Error **errp)
>  {
> -    BlockDeviceInfoList *list, *entry;
> +    BlockDeviceInfoList *list;
>      BlockDriverState *bs;
>
>      list = NULL;
> @@ -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.

>
> -#define QAPI_LIST_ADD(list, element) do { \
> -    typeof(list) _tmp = g_new(typeof(*(list)), 1); \
> -    _tmp->value = (element); \
> -    _tmp->next = (list); \
> -    (list) = _tmp; \
> -} while (0)
> -
>  typedef struct XDbgBlockGraphConstructor {
>      XDbgBlockGraph *graph;
>      GHashTable *graph_nodes;

Reviewed-by: Markus Armbruster <armbru@redhat.com>