aboutsummaryrefslogtreecommitdiff
path: root/qapi
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2014-05-07 09:53:54 +0200
committerLuiz Capitulino <lcapitulino@redhat.com>2014-05-15 14:00:46 -0400
commit297a3646c2947ee64a6d42ca264039732c6218e0 (patch)
tree8ddd334c8e8b42d4972bca08fd57bb79a068cc33 /qapi
parentcdaec3808e756fee3c4e17d0168ec6429eff0dbc (diff)
downloadqemu-arm-297a3646c2947ee64a6d42ca264039732c6218e0.tar.gz
qapi: Replace uncommon use of the error API by the common one
We commonly use the error API like this: err = NULL; foo(..., &err); if (err) { goto out; } bar(..., &err); Every error source is checked separately. The second function is only called when the first one succeeds. Both functions are free to pass their argument to error_set(). Because error_set() asserts no error has been set, this effectively means they must not be called with an error set. The qapi-generated code uses the error API differently: // *errp was initialized to NULL somewhere up the call chain frob(..., errp); gnat(..., errp); Errors accumulate in *errp: first error wins, subsequent errors get dropped. To make this work, the second function does nothing when called with an error set. Requires non-null errp, or else the second function can't see the first one fail. This usage has also bled into visitor tests, and two device model object property getters rtc_get_date() and balloon_stats_get_all(). With the "accumulate" technique, you need fewer error checks in callers, and buy that with an error check in every callee. Can be nice. However, mixing the two techniques is confusing. You can't use the "accumulate" technique with functions designed for the "check separately" technique. You can use the "check separately" technique with functions designed for the "accumulate" technique, but then error_set() can't catch you setting an error more than once. Standardize on the "check separately" technique for now, because it's overwhelmingly prevalent. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Diffstat (limited to 'qapi')
-rw-r--r--qapi/qapi-visit-core.c231
1 files changed, 101 insertions, 130 deletions
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index ffd76372d1..55f8d4068c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -20,28 +20,24 @@
void visit_start_struct(Visitor *v, void **obj, const char *kind,
const char *name, size_t size, Error **errp)
{
- if (!error_is_set(errp)) {
- v->start_struct(v, obj, kind, name, size, errp);
- }
+ v->start_struct(v, obj, kind, name, size, errp);
}
void visit_end_struct(Visitor *v, Error **errp)
{
- assert(!error_is_set(errp));
v->end_struct(v, errp);
}
void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
Error **errp)
{
- if (!error_is_set(errp) && v->start_implicit_struct) {
+ if (v->start_implicit_struct) {
v->start_implicit_struct(v, obj, size, errp);
}
}
void visit_end_implicit_struct(Visitor *v, Error **errp)
{
- assert(!error_is_set(errp));
if (v->end_implicit_struct) {
v->end_implicit_struct(v, errp);
}
@@ -49,30 +45,23 @@ void visit_end_implicit_struct(Visitor *v, Error **errp)
void visit_start_list(Visitor *v, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->start_list(v, name, errp);
- }
+ v->start_list(v, name, errp);
}
GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
{
- if (!error_is_set(errp)) {
- return v->next_list(v, list, errp);
- }
-
- return 0;
+ return v->next_list(v, list, errp);
}
void visit_end_list(Visitor *v, Error **errp)
{
- assert(!error_is_set(errp));
v->end_list(v, errp);
}
void visit_optional(Visitor *v, bool *present, const char *name,
Error **errp)
{
- if (!error_is_set(errp) && v->optional) {
+ if (v->optional) {
v->optional(v, present, name, errp);
}
}
@@ -80,7 +69,7 @@ void visit_optional(Visitor *v, bool *present, const char *name,
void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
const char *name, Error **errp)
{
- if (!error_is_set(errp) && v->get_next_type) {
+ if (v->get_next_type) {
v->get_next_type(v, obj, qtypes, name, errp);
}
}
@@ -88,192 +77,172 @@ void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
void visit_type_enum(Visitor *v, int *obj, const char *strings[],
const char *kind, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->type_enum(v, obj, strings, kind, name, errp);
- }
+ v->type_enum(v, obj, strings, kind, name, errp);
}
void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->type_int(v, obj, name, errp);
- }
+ v->type_int(v, obj, name, errp);
}
void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_uint8) {
- v->type_uint8(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT8_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "uint8_t");
- return;
- }
- *obj = value;
+
+ if (v->type_uint8) {
+ v->type_uint8(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT8_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint8_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_uint16) {
- v->type_uint16(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT16_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "uint16_t");
- return;
- }
- *obj = value;
+
+ if (v->type_uint16) {
+ v->type_uint16(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT16_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint16_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_uint32) {
- v->type_uint32(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT32_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "uint32_t");
- return;
- }
- *obj = value;
+
+ if (v->type_uint32) {
+ v->type_uint32(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT32_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "uint32_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_uint64) {
- v->type_uint64(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- *obj = value;
- }
+
+ if (v->type_uint64) {
+ v->type_uint64(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ *obj = value;
}
}
void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_int8) {
- v->type_int8(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT8_MIN || value > INT8_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "int8_t");
- return;
- }
- *obj = value;
+
+ if (v->type_int8) {
+ v->type_int8(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT8_MIN || value > INT8_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int8_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_int16) {
- v->type_int16(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT16_MIN || value > INT16_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "int16_t");
- return;
- }
- *obj = value;
+
+ if (v->type_int16) {
+ v->type_int16(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT16_MIN || value > INT16_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int16_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_int32) {
- v->type_int32(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT32_MIN || value > INT32_MAX) {
- error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
- "int32_t");
- return;
- }
- *obj = value;
+
+ if (v->type_int32) {
+ v->type_int32(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT32_MIN || value > INT32_MAX) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+ "int32_t");
+ return;
}
+ *obj = value;
}
}
void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- if (v->type_int64) {
- v->type_int64(v, obj, name, errp);
- } else {
- v->type_int(v, obj, name, errp);
- }
+ if (v->type_int64) {
+ v->type_int64(v, obj, name, errp);
+ } else {
+ v->type_int(v, obj, name, errp);
}
}
void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (!error_is_set(errp)) {
- if (v->type_size) {
- v->type_size(v, obj, name, errp);
- } else if (v->type_uint64) {
- v->type_uint64(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- *obj = value;
- }
+
+ if (v->type_size) {
+ v->type_size(v, obj, name, errp);
+ } else if (v->type_uint64) {
+ v->type_uint64(v, obj, name, errp);
+ } else {
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ *obj = value;
}
}
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->type_bool(v, obj, name, errp);
- }
+ v->type_bool(v, obj, name, errp);
}
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->type_str(v, obj, name, errp);
- }
+ v->type_str(v, obj, name, errp);
}
void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
{
- if (!error_is_set(errp)) {
- v->type_number(v, obj, name, errp);
- }
+ v->type_number(v, obj, name, errp);
}
void output_type_enum(Visitor *v, int *obj, const char *strings[],
@@ -299,13 +268,15 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
const char *kind, const char *name,
Error **errp)
{
+ Error *local_err = NULL;
int64_t value = 0;
char *enum_str;
assert(strings);
- visit_type_str(v, &enum_str, name, errp);
- if (error_is_set(errp)) {
+ visit_type_str(v, &enum_str, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
return;
}