[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 10/15] qapi: Clean up fragile use of error_is
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v2 10/15] qapi: Clean up fragile use of error_is_set() |
Date: |
Tue, 29 Apr 2014 16:38:10 -0500 |
User-agent: |
alot/0.3.4 |
Quoting Markus Armbruster (2014-04-28 15:27:49)
> 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 do_qmp_dispatch() is merely fragile, because
> the caller never passes 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 <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Michael Roth <address@hidden>
> ---
> qapi/qmp-dispatch.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 187af56..168b083 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -62,6 +62,7 @@ static QDict *qmp_dispatch_check_obj(const QObject
> *request, Error **errp)
>
> static QObject *do_qmp_dispatch(QObject *request, Error **errp)
> {
> + Error *local_err = NULL;
> const char *command;
> QDict *args, *dict;
> QmpCommand *cmd;
> @@ -93,13 +94,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error
> **errp)
>
> switch (cmd->type) {
> case QCT_NORMAL:
> - cmd->fn(args, &ret, errp);
> - if (!error_is_set(errp)) {
> - if (cmd->options & QCO_NO_SUCCESS_RESP) {
> - g_assert(!ret);
> - } else if (!ret) {
> - ret = QOBJECT(qdict_new());
> - }
> + cmd->fn(args, &ret, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
> + g_assert(!ret);
> + } else if (!ret) {
> + ret = QOBJECT(qdict_new());
> }
> break;
> }
> --
> 1.8.1.4
- [Qemu-devel] [PATCH v2 01/15] qmp hmp: Consistently name Error * objects err, and not errp, (continued)
- [Qemu-devel] [PATCH v2 01/15] qmp hmp: Consistently name Error * objects err, and not errp, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 02/15] qga: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 05/15] error: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 08/15] qapi: Drop redundant, unclean error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 13/15] qemu-option: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 11/15] qga: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 10/15] qapi: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- Re: [Qemu-devel] [PATCH v2 10/15] qapi: Clean up fragile use of error_is_set(),
Michael Roth <=
- [Qemu-devel] [PATCH v2 15/15] qmp: Don't use error_is_set() to suppress additional errors, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 12/15] qga: Drop superfluous error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 07/15] hmp: Guard against misuse of hmp_handle_error(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 09/15] tests/qapi-schema: Drop superfluous error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 14/15] dump: Drop pointless error_is_set(), DumpState member errp, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 06/15] qga: Use return values instead of error_is_set(errp), Markus Armbruster, 2014/04/28