Message ID | 20220316095454.2613871-1-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | Misc fixes and cleanups for 7.0? | expand |
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);
Hi On Wed, Mar 16, 2022 at 4:31 PM Markus Armbruster <armbru@redhat.com> wrote: > > 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. Yeah, unfortunately. There might be other tricks possible, but they would likely be less obvious. > > Style nitpick: a function's opening brace goes on its own line: > > static inline void qbool_unref(QBool *q) > { > qobject_unref(q); > } > right > 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 I agree, I generally don't like inline. However, simple one-liner wrapper kinda fit. I was even tempted to use extra _ at the end of the function to prevent usage outside of the macro, but decided that wouldn't be much better. Btw, what's' your rule for using "static inline" in headers then :) ? > 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); >
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Hi > > On Wed, Mar 16, 2022 at 4:31 PM Markus Armbruster <armbru@redhat.com> wrote: >> >> 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. > > Yeah, unfortunately. There might be other tricks possible, but they > would likely be less obvious. > >> >> Style nitpick: a function's opening brace goes on its own line: >> >> static inline void qbool_unref(QBool *q) >> { >> qobject_unref(q); >> } >> > > right > >> 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 > > I agree, I generally don't like inline. However, simple one-liner > wrapper kinda fit. I was even tempted to use extra _ at the end of the > function to prevent usage outside of the macro, but decided that > wouldn't be much better. > > Btw, what's' your rule for using "static inline" in headers then :) ? Demonstrated performance improvement would be compelling. Occasionally, there's no good .c home for the function, and putting it in the header is overall less bad than creating a .c for it. >> G_DEFINE_AUTOPTR_CLEANUP_FUNC() use a plain extern function. [...]
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) + 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) \ 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);