[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 01/12] qapi: Move GenericList to qapi/util.h
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 01/12] qapi: Move GenericList to qapi/util.h |
Date: |
Mon, 26 Oct 2020 15:18:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> Placing GenericList in util.h will make it easier for the next patch
> to promote QAPI_LIST_ADD() into a public macro without requiring more
> files to include the unrelated visitor.h.
Is this true?
You don't actually need GenericList to make use of QAPI_LIST_ADD(), do
you? Any QAPI list type should do.
> However, we can't also move GenericAlternate; this is because it would
> introduce a circular dependency: qapi-builtin-types.h needs a complete
> definition of QEnumLookup (so it includes qapi/util.h), and
> GenericAlternate needs a complete definition of QType (declared in
> qapi-builtin-types.h). Leaving GenericAlternate in visitor.h breaks
> the cycle, and doesn't matter since we don't have any further planned
> uses for that type outside of visitors.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
I did suggest to consider moving GenericList and GenericAlternate next
to QAPI_LIST_ADD(), because they're (loosely) related. We can't move
GenericAlternate. Moving only GenericList brings GenericList and
QAPI_LIST_ADD() together, but separates the more closely related
GenericList and GenericAlternate. Meh.
I'd leave it put.
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> include/qapi/visitor.h | 9 +--------
> include/qapi/util.h | 8 ++++++++
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index ebc19ede7fff..8c2e1c29ad8b 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -16,6 +16,7 @@
> #define QAPI_VISITOR_H
>
> #include "qapi/qapi-builtin-types.h"
> +#include "qapi/util.h"
Not necessary, qapi-builtin-types.h must include it for QEnumLookup.
> /*
> * The QAPI schema defines both a set of C data types, and a QMP wire
> @@ -228,14 +229,6 @@
>
> /*** Useful types ***/
>
> -/* This struct is layout-compatible with all other *List structs
> - * created by the QAPI generator. It is used as a typical
> - * singly-linked list. */
> -typedef struct GenericList {
> - struct GenericList *next;
> - char padding[];
> -} GenericList;
> -
> /* This struct is layout-compatible with all Alternate types
> * created by the QAPI generator. */
> typedef struct GenericAlternate {
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index a7c3c6414874..50201896c7a4 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -11,6 +11,14 @@
> #ifndef QAPI_UTIL_H
> #define QAPI_UTIL_H
>
> +/* This struct is layout-compatible with all other *List structs
> + * created by the QAPI generator. It is used as a typical
> + * singly-linked list. */
> +typedef struct GenericList {
> + struct GenericList *next;
> + char padding[];
> +} GenericList;
> +
> typedef struct QEnumLookup {
> const char *const *array;
> int size;