[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 02/23] qapi: Guarantee NULL obj on input visitor call
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL 02/23] qapi: Guarantee NULL obj on input visitor callback error |
Date: |
Thu, 12 May 2016 09:58:49 +0200 |
From: Eric Blake <address@hidden>
Our existing input visitors were not very consistent on errors in a
function taking 'TYPE **obj'. These are start_struct(),
start_alternate(), type_str(), and type_any(). next_list() is
similar, but can't fail (see commit 08f9541). While all of them set
'*obj' to allocated storage on success, it was not obvious whether
'*obj' was guaranteed safe on failure, or whether it was left
uninitialized. But a future patch wants to guarantee that
visit_type_FOO() does not leak a partially-constructed obj back to
the caller; it is easier to implement this if we can reliably state
that input visitors assign '*obj' regardless of success or failure,
and that on failure *obj is NULL. Add assertions to enforce
consistency in the final setting of err vs. *obj.
The opts-visitor start_struct() doesn't set an error, but it
also was doing a weird check for 0 size; all callers pass in
non-zero size if obj is non-NULL.
The testsuite has at least one spot where we no longer need
to pre-initialize a variable prior to a visit; valgrind confirms
that the test is still fine with the cleanup.
A later patch will document the design constraint implemented
here.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[visit_start_alternate()'s assertion tightened, commit message tweaked]
Signed-off-by: Markus Armbruster <address@hidden>
---
qapi/opts-visitor.c | 3 ++-
qapi/qapi-visit-core.c | 34 ++++++++++++++++++++++++++++++----
qapi/qmp-input-visitor.c | 4 ++++
qapi/string-input-visitor.c | 1 +
tests/test-qmp-input-strict.c | 2 +-
5 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 66aeaed..4cb6436 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -133,7 +133,7 @@ opts_start_struct(Visitor *v, const char *name, void **obj,
const QemuOpt *opt;
if (obj) {
- *obj = g_malloc0(size > 0 ? size : 1);
+ *obj = g_malloc0(size);
}
if (ov->depth++ > 0) {
return;
@@ -314,6 +314,7 @@ opts_type_str(Visitor *v, const char *name, char **obj,
Error **errp)
opt = lookup_scalar(ov, name, errp);
if (!opt) {
+ *obj = NULL;
return;
}
*obj = g_strdup(opt->str ? opt->str : "");
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 3cd7edc..528092e 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -23,7 +23,13 @@
void visit_start_struct(Visitor *v, const char *name, void **obj,
size_t size, Error **errp)
{
- v->start_struct(v, name, obj, size, errp);
+ Error *err = NULL;
+
+ v->start_struct(v, name, obj, size, &err);
+ if (obj && v->type == VISITOR_INPUT) {
+ assert(!err != !*obj);
+ }
+ error_propagate(errp, err);
}
void visit_end_struct(Visitor *v, Error **errp)
@@ -51,10 +57,16 @@ void visit_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
bool promote_int, Error **errp)
{
+ Error *err = NULL;
+
assert(obj && size >= sizeof(GenericAlternate));
if (v->start_alternate) {
- v->start_alternate(v, name, obj, size, promote_int, errp);
+ v->start_alternate(v, name, obj, size, promote_int, &err);
}
+ if (v->type == VISITOR_INPUT) {
+ assert(v->start_alternate && !err != !*obj);
+ }
+ error_propagate(errp, err);
}
void visit_end_alternate(Visitor *v)
@@ -188,7 +200,14 @@ void visit_type_bool(Visitor *v, const char *name, bool
*obj, Error **errp)
void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
{
- v->type_str(v, name, obj, errp);
+ Error *err = NULL;
+
+ assert(obj);
+ v->type_str(v, name, obj, &err);
+ if (v->type == VISITOR_INPUT) {
+ assert(!err != !*obj);
+ }
+ error_propagate(errp, err);
}
void visit_type_number(Visitor *v, const char *name, double *obj,
@@ -199,7 +218,14 @@ void visit_type_number(Visitor *v, const char *name,
double *obj,
void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp)
{
- v->type_any(v, name, obj, errp);
+ Error *err = NULL;
+
+ assert(obj);
+ v->type_any(v, name, obj, &err);
+ if (v->type == VISITOR_INPUT) {
+ assert(!err != !*obj);
+ }
+ error_propagate(errp, err);
}
static void output_type_enum(Visitor *v, const char *name, int *obj,
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 02d4233..77cce8b 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -120,6 +120,9 @@ static void qmp_input_start_struct(Visitor *v, const char
*name, void **obj,
QObject *qobj = qmp_input_get_object(qiv, name, true);
Error *err = NULL;
+ if (obj) {
+ *obj = NULL;
+ }
if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"QDict");
@@ -267,6 +270,7 @@ static void qmp_input_type_str(Visitor *v, const char
*name, char **obj,
QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
if (!qstr) {
+ *obj = NULL;
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"string");
return;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 97009a6..b55d199 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -293,6 +293,7 @@ static void parse_type_str(Visitor *v, const char *name,
char **obj,
if (siv->string) {
*obj = g_strdup(siv->string);
} else {
+ *obj = NULL;
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"string");
}
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index d71727e..d5f80ec 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -263,7 +263,7 @@ static void
test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
static void test_validate_fail_alternate(TestInputVisitorData *data,
const void *unused)
{
- UserDefAlternate *tmp = NULL;
+ UserDefAlternate *tmp;
Visitor *v;
Error *err = NULL;
--
2.5.5
- [Qemu-devel] [PULL 00/23] QAPI patches for 2016-05-12, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 09/23] qom: Wrap prop visit in visit_start_struct, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 10/23] qmp-input: Require struct push to visit members of top dict, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 03/23] qmp: Drop dead command->type, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 01/23] qapi-visit: Add visitor.type classification, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 06/23] qapi: Use strict QMP input visitor in more places, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 07/23] qmp-input: Don't consume input when checking has_member, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 18/23] qmp: Tighten output visitor rules, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 05/23] qapi: Consolidate QMP input visitor creation, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 02/23] qapi: Guarantee NULL obj on input visitor callback error,
Markus Armbruster <=
- [Qemu-devel] [PULL 21/23] qapi: Fix string input visitor handling of invalid list, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 20/23] tests/string-input-visitor: Add negative integer tests, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 14/23] qapi: Add visit_type_null() visitor, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 17/23] qmp: Don't reuse qmp visitor after grabbing output, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 13/23] tests: Add check-qnull, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 23/23] qapi: Change visit_type_FOO() to no longer return partial objects, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 04/23] qmp-input: Clean up stack handling, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 11/23] qmp-input: Refactor when list is advanced, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 16/23] spapr_drc: Expose 'null' in qom-get when there is no fdt, Markus Armbruster, 2016/05/12
- [Qemu-devel] [PULL 22/23] qapi: Simplify semantics of visit_next_list(), Markus Armbruster, 2016/05/12