| Message ID | 1400518658-2515-3-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/19/2014 10:57 AM, Markus Armbruster wrote: > Using error_is_set(ERRP) to find out whether a function failed is > either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP > may be null, because errors go undetected when it is. It's fragile > when proving ERRP non-null involves a non-local argument. Else, it's > unnecessarily opaque (see commit 84d18f0). > > The error_is_set(errp) in qemu_chr_new_from_opts() is merely fragile, > because the callers never pass a null errp argument. > > Make the code more robust and more obviously correct: receive the > error in a local variable, then propagate it through the parameter. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qemu-char.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/qemu-char.c b/qemu-char.c index 3eaefc9..5a7975f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3204,6 +3204,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, void (*init)(struct CharDriverState *s), Error **errp) { + Error *local_err = NULL; CharDriver *cd; CharDriverState *chr; GSList *i; @@ -3245,8 +3246,9 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, chr = NULL; backend->kind = cd->kind; if (cd->parse) { - cd->parse(opts, backend, errp); - if (error_is_set(errp)) { + cd->parse(opts, backend, &local_err); + if (local_err) { + error_propagate(errp, local_err); goto qapi_out; } }
Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). The error_is_set(errp) in qemu_chr_new_from_opts() is merely fragile, because the callers never pass a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qemu-char.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)