Re: [PATCH 25/27] include/qapi: add g_autoptr support for qobject types

[Markus Armbruster <armbru@redhat.com>]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add small inline wrappers for qobject_unref() calls, which is a macro.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qbool.h   | 6 ++++++
>  include/qapi/qmp/qdict.h   | 6 ++++++
>  include/qapi/qmp/qlist.h   | 8 +++++++-
>  include/qapi/qmp/qnull.h   | 6 ++++++
>  include/qapi/qmp/qnum.h    | 6 ++++++
>  include/qapi/qmp/qstring.h | 6 ++++++
>  6 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
> index 2f888d10573f..52b1c5c15280 100644
> --- a/include/qapi/qmp/qbool.h
> +++ b/include/qapi/qmp/qbool.h
> @@ -21,6 +21,12 @@ struct QBool {
>      bool value;
>  };
>  
> +static inline void qbool_unref(QBool *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QBool, qbool_unref)
> +

You need the wrapper function around the wrapper macro qobject_unref(),
because

   G_DEFINE_AUTOPTR_CLEANUP_FUNC(QBool, qobject_unref_impl)

dies with "passing argument 1 of ‘qobject_unref_impl’ from incompatible
pointer type [-Wincompatible-pointer-types]".  Okay.

Style nitpick: a function's opening brace goes on its own line:

   static inline void qbool_unref(QBool *q)
   {
       qobject_unref(q);
   }

Moreover, I prefer to put code in headers only when there's a real need.
I don't see one here.  Most existing uses of
G_DEFINE_AUTOPTR_CLEANUP_FUNC() use a plain extern function.

>  QBool *qbool_from_bool(bool value);
>  bool qbool_get_bool(const QBool *qb);
>  
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index d5b5430e21a9..9f0a6a6708b5 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -30,6 +30,12 @@ struct QDict {
>      QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
>  };
>  
> +static inline void qdict_unref(QDict *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QDict, qdict_unref)
> +
>  /* Object API */
>  QDict *qdict_new(void);
>  const char *qdict_entry_key(const QDictEntry *entry);
> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> index 06e98ad5f498..06c267dfb898 100644
> --- a/include/qapi/qmp/qlist.h
> +++ b/include/qapi/qmp/qlist.h
> @@ -26,7 +26,13 @@ struct QList {
>      QTAILQ_HEAD(,QListEntry) head;
>  };
>  
> -#define qlist_append(qlist, obj) \
> +static inline void qlist_unref(QList *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QList, qlist_unref)
> +
> +#define qlist_append(qlist, obj)                \

The whitespace change looks accidental.

>          qlist_append_obj(qlist, QOBJECT(obj))
>  
>  void qlist_append_bool(QList *qlist, bool value);
> diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
> index e84ecceedbcb..8c45e08b1c47 100644
> --- a/include/qapi/qmp/qnull.h
> +++ b/include/qapi/qmp/qnull.h
> @@ -26,4 +26,10 @@ static inline QNull *qnull(void)
>      return qobject_ref(&qnull_);
>  }
>  
> +static inline void qnull_unref(QNull *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QNull, qnull_unref)
> +
>  #endif /* QNULL_H */
> diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
> index 7f84e20bfb2c..ebbf9cd5abe8 100644
> --- a/include/qapi/qmp/qnum.h
> +++ b/include/qapi/qmp/qnum.h
> @@ -54,6 +54,12 @@ struct QNum {
>      } u;
>  };
>  
> +static inline void qnum_unref(QNum *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QNum, qnum_unref)
> +
>  QNum *qnum_from_int(int64_t value);
>  QNum *qnum_from_uint(uint64_t value);
>  QNum *qnum_from_double(double value);
> diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
> index 1d8ba469368f..a38d2925d757 100644
> --- a/include/qapi/qmp/qstring.h
> +++ b/include/qapi/qmp/qstring.h
> @@ -20,6 +20,12 @@ struct QString {
>      const char *string;
>  };
>  
> +static inline void qstring_unref(QString *q) {
> +    qobject_unref(q);
> +}
> +
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(QString, qstring_unref)
> +
>  QString *qstring_new(void);
>  QString *qstring_from_str(const char *str);
>  QString *qstring_from_substr(const char *str, size_t start, size_t end);