Re: [Qemu-devel] [PATCH v3 3/4] qobject: replace qobject_incref/QINCREF qobject_decref/QDECREF

From: Eric Blake <eblake@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>, qemu-devel@nongnu.org
Cc: berrange@redhat.com, armbru@redhat.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH v3 3/4] qobject: replace qobject_incref/QINCREF qobject_decref/QDECREF
Date: Thu, 29 Mar 2018 11:23:26 -0500	[thread overview]
Message-ID: <b041bde4-b926-2ede-d8a5-4721dafe6736@redhat.com> (raw)
In-Reply-To: <20180329154833.566-4-marcandre.lureau@redhat.com>

On 03/29/2018 10:48 AM, Marc-André Lureau wrote:
> Now that we can safely call QOBJECT() on QObject * and children types,
> we can have a single macro to ref/unref the object.
> 
> Change the incref/decref prefix name for the more common ref/unref.
> 
> Note that before the patch, "QDECREF(obj)" was problematic because it
> would expand to "obj ? obj : ...", which could evaluate "obj" multiple
> times.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---

> +++ b/include/qapi/qmp/qobject.h
> @@ -48,14 +48,6 @@ struct QObject {
>       __x ? container_of(&(__x)->base, QObject, base) : NULL; \
>   })
>   
> -/* High-level interface for qobject_incref() */
> -#define QINCREF(obj)      \
> -    qobject_incref(QOBJECT(obj))
> -
> -/* High-level interface for qobject_decref() */
> -#define QDECREF(obj)              \
> -    qobject_decref(obj ? QOBJECT(obj) : NULL)
> -

Interesting choice to move the macros...

>   /* Required for qobject_to() */
>   #define QTYPE_CAST_TO_QNull     QTYPE_QNULL
>   #define QTYPE_CAST_TO_QNum      QTYPE_QNUM
> @@ -78,10 +70,7 @@ static inline void qobject_init(QObject *obj, QType type)
>       obj->base.type = type;
>   }
>   
> -/**
> - * qobject_incref(): Increment QObject's reference count
> - */
> -static inline void qobject_incref(QObject *obj)
> +static inline void qobject_ref(QObject *obj)
>   {
>       if (obj) {
>           obj->base.refcnt++;
> @@ -102,11 +91,7 @@ bool qobject_is_equal(const QObject *x, const QObject *y);
>    */
>   void qobject_destroy(QObject *obj);
>   
> -/**
> - * qobject_decref(): Decrement QObject's reference count, deallocate
> - * when it reaches zero
> - */
> -static inline void qobject_decref(QObject *obj)
> +static inline void qobject_unref(QObject *obj)
>   {
>       assert(!obj || obj->base.refcnt);
>       if (obj && --obj->base.refcnt == 0) {
> @@ -114,6 +99,19 @@ static inline void qobject_decref(QObject *obj)
>       }
>   }
>   
> +/**
> + * qobject_ref(): Increment QObject's reference count
> + */
> +#define qobject_ref(obj)                        \
> +    qobject_ref(QOBJECT(obj))

...below the functions of the same name.  C preprocessor rules guarantee 
that you don't get infinite expansion, although I did a double-take the 
first time I read through the patch (especially since your v2 used the 
name qobject_ref_impl() for the function, distinct from the macro name). 
  Worth a comment?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org