qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null
@ 2015-10-15 14:15 Markus Armbruster
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD Markus Armbruster
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

The qobject_to_FOO() crash on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.

Throw in a patch to drop QObject_HEAD.

Luiz, I'm happy to take this through my tree, since got a QMP series
based on it (to be posted shortly).

Markus Armbruster (6):
  qobject: Drop QObject_HEAD
  qbool: Make conversion from QObject * accept null
  qdict: Make conversion from QObject * accept null
  qfloat qint: Make conversion from QObject * accept null
  qlist: Make conversion from QObject * accept null
  qstring: Make conversion from QObject * accept null

 include/qapi/qmp/qbool.h   |  2 +-
 include/qapi/qmp/qdict.h   |  2 +-
 include/qapi/qmp/qfloat.h  |  2 +-
 include/qapi/qmp/qint.h    |  2 +-
 include/qapi/qmp/qlist.h   |  2 +-
 include/qapi/qmp/qobject.h |  4 ----
 include/qapi/qmp/qstring.h |  2 +-
 qapi/qmp-input-visitor.c   | 40 ++++++++++++++++++++++------------------
 qga/main.c                 | 11 +++--------
 qobject/qbool.c            |  4 ++--
 qobject/qdict.c            | 39 ++++++++++++---------------------------
 qobject/qfloat.c           |  4 ++--
 qobject/qint.c             |  4 ++--
 qobject/qlist.c            |  3 +--
 qobject/qstring.c          |  4 ++--
 15 files changed, 52 insertions(+), 73 deletions(-)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 15:24   ` Eric Blake
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null Markus Armbruster
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

QObject_HEAD is a macro expanding into the common part of structs that
are sub-types of QObject.  It's always been just QObject base, and
unlikely to change.  Drop the macro, because the code is clearer with
out it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qbool.h   | 2 +-
 include/qapi/qmp/qdict.h   | 2 +-
 include/qapi/qmp/qfloat.h  | 2 +-
 include/qapi/qmp/qint.h    | 2 +-
 include/qapi/qmp/qlist.h   | 2 +-
 include/qapi/qmp/qobject.h | 4 ----
 include/qapi/qmp/qstring.h | 2 +-
 7 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index 4aa6be3..d9256e4 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qobject.h"
 
 typedef struct QBool {
-    QObject_HEAD;
+    QObject base;
     bool value;
 } QBool;
 
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index a37f4c1..787c658 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -28,7 +28,7 @@ typedef struct QDictEntry {
 } QDictEntry;
 
 typedef struct QDict {
-    QObject_HEAD;
+    QObject base;
     size_t size;
     QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
 } QDict;
diff --git a/include/qapi/qmp/qfloat.h b/include/qapi/qmp/qfloat.h
index a865844..46745e5 100644
--- a/include/qapi/qmp/qfloat.h
+++ b/include/qapi/qmp/qfloat.h
@@ -18,7 +18,7 @@
 #include "qapi/qmp/qobject.h"
 
 typedef struct QFloat {
-    QObject_HEAD;
+    QObject base;
     double value;
 } QFloat;
 
diff --git a/include/qapi/qmp/qint.h b/include/qapi/qmp/qint.h
index 48a41b0..339a9ab 100644
--- a/include/qapi/qmp/qint.h
+++ b/include/qapi/qmp/qint.h
@@ -17,7 +17,7 @@
 #include "qapi/qmp/qobject.h"
 
 typedef struct QInt {
-    QObject_HEAD;
+    QObject base;
     int64_t value;
 } QInt;
 
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 6cc4831..b1bf785 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -22,7 +22,7 @@ typedef struct QListEntry {
 } QListEntry;
 
 typedef struct QList {
-    QObject_HEAD;
+    QObject base;
     QTAILQ_HEAD(,QListEntry) head;
 } QList;
 
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 260d2ed..c856f55 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -59,10 +59,6 @@ typedef struct QObject {
     size_t refcnt;
 } QObject;
 
-/* Objects definitions must include this */
-#define QObject_HEAD  \
-    QObject base
-
 /* Get the 'base' part of an object */
 #define QOBJECT(obj) (&(obj)->base)
 
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 1bc3666..34675a7 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -17,7 +17,7 @@
 #include "qapi/qmp/qobject.h"
 
 typedef struct QString {
-    QObject_HEAD;
+    QObject base;
     char *string;
     size_t length;
     size_t capacity;
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 15:27   ` Eric Blake
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 3/6] qdict: " Markus Armbruster
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

qobject_to_qbool() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/qmp-input-visitor.c |  6 +++---
 qobject/qbool.c          |  4 ++--
 qobject/qdict.c          | 11 +++--------
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5dd9ed5..f32ce81 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -240,15 +240,15 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
                                 Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
 
-    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
+    if (!qbool) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "boolean");
         return;
     }
 
-    *obj = qbool_get_bool(qobject_to_qbool(qobj));
+    *obj = qbool_get_bool(qbool);
 }
 
 static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
diff --git a/qobject/qbool.c b/qobject/qbool.c
index 5ff69f0..bc6535f 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -51,9 +51,9 @@ bool qbool_get_bool(const QBool *qb)
  */
 QBool *qobject_to_qbool(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QBOOL)
+    if (!obj || qobject_type(obj) != QTYPE_QBOOL) {
         return NULL;
-
+    }
     return container_of(obj, QBool, base);
 }
 
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 67b1a58..f179f4e 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -243,8 +243,7 @@ int64_t qdict_get_int(const QDict *qdict, const char *key)
  */
 bool qdict_get_bool(const QDict *qdict, const char *key)
 {
-    QObject *obj = qdict_get_obj(qdict, key, QTYPE_QBOOL);
-    return qbool_get_bool(qobject_to_qbool(obj));
+    return qbool_get_bool(qobject_to_qbool(qdict_get(qdict, key)));
 }
 
 /**
@@ -316,13 +315,9 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
  */
 bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value)
 {
-    QObject *obj;
+    QBool *qbool = qobject_to_qbool(qdict_get(qdict, key));
 
-    obj = qdict_get(qdict, key);
-    if (!obj || qobject_type(obj) != QTYPE_QBOOL)
-        return def_value;
-
-    return qbool_get_bool(qobject_to_qbool(obj));
+    return qbool ? qbool_get_bool(qbool) : def_value;
 }
 
 /**
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 3/6] qdict: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD Markus Armbruster
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 15:30   ` Eric Blake
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 4/6] qfloat qint: " Markus Armbruster
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

qobject_to_qdict() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/main.c      | 11 +++--------
 qobject/qdict.c |  6 +++---
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index d8e063a..4eb0310 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -567,7 +567,6 @@ static void process_command(GAState *s, QDict *req)
 static void process_event(JSONMessageParser *parser, QList *tokens)
 {
     GAState *s = container_of(parser, GAState, parser);
-    QObject *obj;
     QDict *qdict;
     Error *err = NULL;
     int ret;
@@ -575,9 +574,9 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
     g_assert(s && parser);
 
     g_debug("process_event: called");
-    obj = json_parser_parse_err(tokens, NULL, &err);
-    if (err || !obj || qobject_type(obj) != QTYPE_QDICT) {
-        qobject_decref(obj);
+    qdict = qobject_to_qdict(json_parser_parse_err(tokens, NULL, &err));
+    if (err || !qdict) {
+        QDECREF(qdict);
         qdict = qdict_new();
         if (!err) {
             g_warning("failed to parse event: unknown error");
@@ -587,12 +586,8 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
         }
         qdict_put_obj(qdict, "error", qmp_build_error_object(err));
         error_free(err);
-    } else {
-        qdict = qobject_to_qdict(obj);
     }
 
-    g_assert(qdict);
-
     /* handle host->guest commands */
     if (qdict_haskey(qdict, "execute")) {
         process_command(s, qdict);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index f179f4e..6b32285 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -46,9 +46,9 @@ QDict *qdict_new(void)
  */
 QDict *qobject_to_qdict(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QDICT)
+    if (!obj || qobject_type(obj) != QTYPE_QDICT) {
         return NULL;
-
+    }
     return container_of(obj, QDict, base);
 }
 
@@ -269,7 +269,7 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key)
  */
 QDict *qdict_get_qdict(const QDict *qdict, const char *key)
 {
-    return qobject_to_qdict(qdict_get_obj(qdict, key, QTYPE_QDICT));
+    return qobject_to_qdict(qdict_get(qdict, key));
 }
 
 /**
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 4/6] qfloat qint: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
                   ` (2 preceding siblings ...)
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 3/6] qdict: " Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 16:09   ` Eric Blake
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 5/6] qlist: " Markus Armbruster
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

qobject_to_qfloat() and qobject_to_qint() crash on null, which is a
trap for the unwary.  Return null instead, and simplify a few callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/qmp-input-visitor.c | 28 ++++++++++++++++------------
 qobject/qdict.c          | 11 +++--------
 qobject/qfloat.c         |  4 ++--
 qobject/qint.c           |  4 ++--
 4 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index f32ce81..267783c 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -225,15 +225,15 @@ static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
                                Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
 
-    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
+    if (!qint) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "integer");
         return;
     }
 
-    *obj = qint_get_int(qobject_to_qint(qobj));
+    *obj = qint_get_int(qint);
 }
 
 static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
@@ -271,19 +271,23 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QInt *qint;
+    QFloat *qfloat;
 
-    if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT &&
-        qobject_type(qobj) != QTYPE_QINT)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "number");
-        return;
-    }
-
-    if (qobject_type(qobj) == QTYPE_QINT) {
+    qint = qobject_to_qint(qobj);
+    if (qint) {
         *obj = qint_get_int(qobject_to_qint(qobj));
-    } else {
+        return;
+    }
+
+    qfloat = qobject_to_qfloat(qobj);
+    if (qfloat) {
         *obj = qfloat_get_double(qobject_to_qfloat(qobj));
+        return;
     }
+
+    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+               "number");
 }
 
 static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 6b32285..97e881b 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -229,8 +229,7 @@ double qdict_get_double(const QDict *qdict, const char *key)
  */
 int64_t qdict_get_int(const QDict *qdict, const char *key)
 {
-    QObject *obj = qdict_get_obj(qdict, key, QTYPE_QINT);
-    return qint_get_int(qobject_to_qint(obj));
+    return qint_get_int(qobject_to_qint(qdict_get(qdict, key)));
 }
 
 /**
@@ -297,13 +296,9 @@ const char *qdict_get_str(const QDict *qdict, const char *key)
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t def_value)
 {
-    QObject *obj;
+    QInt *qint = qobject_to_qint(qdict_get(qdict, key));
 
-    obj = qdict_get(qdict, key);
-    if (!obj || qobject_type(obj) != QTYPE_QINT)
-        return def_value;
-
-    return qint_get_int(qobject_to_qint(obj));
+    return qint ? qint_get_int(qint) : def_value;
 }
 
 /**
diff --git a/qobject/qfloat.c b/qobject/qfloat.c
index 7de0992..c865163 100644
--- a/qobject/qfloat.c
+++ b/qobject/qfloat.c
@@ -51,9 +51,9 @@ double qfloat_get_double(const QFloat *qf)
  */
 QFloat *qobject_to_qfloat(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QFLOAT)
+    if (!obj || qobject_type(obj) != QTYPE_QFLOAT) {
         return NULL;
-
+    }
     return container_of(obj, QFloat, base);
 }
 
diff --git a/qobject/qint.c b/qobject/qint.c
index 86b9b04..999688e 100644
--- a/qobject/qint.c
+++ b/qobject/qint.c
@@ -50,9 +50,9 @@ int64_t qint_get_int(const QInt *qi)
  */
 QInt *qobject_to_qint(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QINT)
+    if (!obj || qobject_type(obj) != QTYPE_QINT) {
         return NULL;
-
+    }
     return container_of(obj, QInt, base);
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 5/6] qlist: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
                   ` (3 preceding siblings ...)
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 4/6] qfloat qint: " Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 16:10   ` Eric Blake
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 6/6] qstring: " Markus Armbruster
  2015-10-15 16:06 ` [Qemu-devel] [PATCH 0/6] qobject: " Luiz Capitulino
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

qobject_to_qlist() crashes on null, which is a trap for the unwary.
Return null instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qlist.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qobject/qlist.c b/qobject/qlist.c
index 1ced0de..298003a 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -142,10 +142,9 @@ size_t qlist_size(const QList *qlist)
  */
 QList *qobject_to_qlist(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QLIST) {
+    if (!obj || qobject_type(obj) != QTYPE_QLIST) {
         return NULL;
     }
-
     return container_of(obj, QList, base);
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 6/6] qstring: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
                   ` (4 preceding siblings ...)
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 5/6] qlist: " Markus Armbruster
@ 2015-10-15 14:15 ` Markus Armbruster
  2015-10-15 16:11   ` Eric Blake
  2015-10-15 16:06 ` [Qemu-devel] [PATCH 0/6] qobject: " Luiz Capitulino
  6 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-10-15 14:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: mdroth, lcapitulino

qobject_to_qstring() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/qmp-input-visitor.c |  6 +++---
 qobject/qdict.c          | 11 +++--------
 qobject/qstring.c        |  4 ++--
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 267783c..eb6e110 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -255,15 +255,15 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
                                Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qmp_input_get_object(qiv, name, true);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
 
-    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
+    if (!qstr) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "string");
         return;
     }
 
-    *obj = g_strdup(qstring_get_str(qobject_to_qstring(qobj)));
+    *obj = g_strdup(qstring_get_str(qstr));
 }
 
 static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 97e881b..2d67bf1 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -282,8 +282,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key)
  */
 const char *qdict_get_str(const QDict *qdict, const char *key)
 {
-    QObject *obj = qdict_get_obj(qdict, key, QTYPE_QSTRING);
-    return qstring_get_str(qobject_to_qstring(obj));
+    return qstring_get_str(qobject_to_qstring(qdict_get(qdict, key)));
 }
 
 /**
@@ -325,13 +324,9 @@ bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value)
  */
 const char *qdict_get_try_str(const QDict *qdict, const char *key)
 {
-    QObject *obj;
+    QString *qstr = qobject_to_qstring(qdict_get(qdict, key));
 
-    obj = qdict_get(qdict, key);
-    if (!obj || qobject_type(obj) != QTYPE_QSTRING)
-        return NULL;
-
-    return qstring_get_str(qobject_to_qstring(obj));
+    return qstr ? qstring_get_str(qstr) : NULL;
 }
 
 /**
diff --git a/qobject/qstring.c b/qobject/qstring.c
index 607b7a1..cb72dfb 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -117,9 +117,9 @@ void qstring_append_chr(QString *qstring, int c)
  */
 QString *qobject_to_qstring(const QObject *obj)
 {
-    if (qobject_type(obj) != QTYPE_QSTRING)
+    if (!obj || qobject_type(obj) != QTYPE_QSTRING) {
         return NULL;
-
+    }
     return container_of(obj, QString, base);
 }
 
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD Markus Armbruster
@ 2015-10-15 15:24   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 15:24 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> QObject_HEAD is a macro expanding into the common part of structs that
> are sub-types of QObject.  It's always been just QObject base, and
> unlikely to change.  Drop the macro, because the code is clearer with
> out it.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qbool.h   | 2 +-
>  include/qapi/qmp/qdict.h   | 2 +-
>  include/qapi/qmp/qfloat.h  | 2 +-
>  include/qapi/qmp/qint.h    | 2 +-
>  include/qapi/qmp/qlist.h   | 2 +-
>  include/qapi/qmp/qobject.h | 4 ----
>  include/qapi/qmp/qstring.h | 2 +-
>  7 files changed, 6 insertions(+), 10 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null Markus Armbruster
@ 2015-10-15 15:27   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 15:27 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> qobject_to_qbool() crashes on null, which is a trap for the unwary.
> Return null instead, and simplify a few callers.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/qmp-input-visitor.c |  6 +++---
>  qobject/qbool.c          |  4 ++--
>  qobject/qdict.c          | 11 +++--------
>  3 files changed, 8 insertions(+), 13 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] qdict: Make conversion from QObject * accept null
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 3/6] qdict: " Markus Armbruster
@ 2015-10-15 15:30   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 15:30 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> qobject_to_qdict() crashes on null, which is a trap for the unwary.
> Return null instead, and simplify a few callers.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/main.c      | 11 +++--------
>  qobject/qdict.c |  6 +++---
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null
  2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
                   ` (5 preceding siblings ...)
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 6/6] qstring: " Markus Armbruster
@ 2015-10-15 16:06 ` Luiz Capitulino
  2015-10-22 12:20   ` Markus Armbruster
  6 siblings, 1 reply; 15+ messages in thread
From: Luiz Capitulino @ 2015-10-15 16:06 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, mdroth

On Thu, 15 Oct 2015 16:15:31 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> The qobject_to_FOO() crash on null, which is a trap for the unwary.
> Return null instead, and simplify a few callers.
> 
> Throw in a patch to drop QObject_HEAD.
> 
> Luiz, I'm happy to take this through my tree, since got a QMP series
> based on it (to be posted shortly).

Please do.

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

Btw, what do you think about this patch? :)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9bd2b8f..aa03f3d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1010,7 +1010,7 @@ F: qemu-timer.c
 F: vl.c
 
 Human Monitor (HMP)
-M: Luiz Capitulino <lcapitulino@redhat.com>
+M: Markus Armbruster <armbru@redhat.com>
 S: Maintained
 F: monitor.c
 F: hmp.c
@@ -1073,7 +1073,7 @@ F: qapi/*.json
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
 QObject
-M: Luiz Capitulino <lcapitulino@redhat.com>
+M: Markus Armbruster <armbru@redhat.com>
 S: Maintained
 F: qobject/
 F: include/qapi/qmp/

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 4/6] qfloat qint: Make conversion from QObject * accept null
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 4/6] qfloat qint: " Markus Armbruster
@ 2015-10-15 16:09   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 16:09 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> qobject_to_qfloat() and qobject_to_qint() crash on null, which is a
> trap for the unwary.  Return null instead, and simplify a few callers.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/qmp-input-visitor.c | 28 ++++++++++++++++------------
>  qobject/qdict.c          | 11 +++--------
>  qobject/qfloat.c         |  4 ++--
>  qobject/qint.c           |  4 ++--
>  4 files changed, 23 insertions(+), 24 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 5/6] qlist: Make conversion from QObject * accept null
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 5/6] qlist: " Markus Armbruster
@ 2015-10-15 16:10   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 16:10 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> qobject_to_qlist() crashes on null, which is a trap for the unwary.
> Return null instead.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qobject/qlist.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 6/6] qstring: Make conversion from QObject * accept null
  2015-10-15 14:15 ` [Qemu-devel] [PATCH 6/6] qstring: " Markus Armbruster
@ 2015-10-15 16:11   ` Eric Blake
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-10-15 16:11 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mdroth, lcapitulino

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On 10/15/2015 08:15 AM, Markus Armbruster wrote:
> qobject_to_qstring() crashes on null, which is a trap for the unwary.
> Return null instead, and simplify a few callers.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/qmp-input-visitor.c |  6 +++---
>  qobject/qdict.c          | 11 +++--------
>  qobject/qstring.c        |  4 ++--
>  3 files changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null
  2015-10-15 16:06 ` [Qemu-devel] [PATCH 0/6] qobject: " Luiz Capitulino
@ 2015-10-22 12:20   ` Markus Armbruster
  0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-10-22 12:20 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: qemu-devel, mdroth

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 15 Oct 2015 16:15:31 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> The qobject_to_FOO() crash on null, which is a trap for the unwary.
>> Return null instead, and simplify a few callers.
>> 
>> Throw in a patch to drop QObject_HEAD.
>> 
>> Luiz, I'm happy to take this through my tree, since got a QMP series
>> based on it (to be posted shortly).
>
> Please do.
>
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

Thanks!

> Btw, what do you think about this patch? :)

I knew I was playing with fire %-}

Right now I'm too swamped with QAPI patches to take on more.  Once we
flushed that queue, and got QAPI to quiet down a bit, I can take HMP and
QObject off you.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-10-22 12:20 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15 14:15 [Qemu-devel] [PATCH 0/6] qobject: Make conversion from QObject * accept null Markus Armbruster
2015-10-15 14:15 ` [Qemu-devel] [PATCH 1/6] qobject: Drop QObject_HEAD Markus Armbruster
2015-10-15 15:24   ` Eric Blake
2015-10-15 14:15 ` [Qemu-devel] [PATCH 2/6] qbool: Make conversion from QObject * accept null Markus Armbruster
2015-10-15 15:27   ` Eric Blake
2015-10-15 14:15 ` [Qemu-devel] [PATCH 3/6] qdict: " Markus Armbruster
2015-10-15 15:30   ` Eric Blake
2015-10-15 14:15 ` [Qemu-devel] [PATCH 4/6] qfloat qint: " Markus Armbruster
2015-10-15 16:09   ` Eric Blake
2015-10-15 14:15 ` [Qemu-devel] [PATCH 5/6] qlist: " Markus Armbruster
2015-10-15 16:10   ` Eric Blake
2015-10-15 14:15 ` [Qemu-devel] [PATCH 6/6] qstring: " Markus Armbruster
2015-10-15 16:11   ` Eric Blake
2015-10-15 16:06 ` [Qemu-devel] [PATCH 0/6] qobject: " Luiz Capitulino
2015-10-22 12:20   ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).