diff options
| author | Markus Armbruster <armbru@redhat.com> | 2014-04-25 16:50:34 +0200 |
|---|---|---|
| committer | Stefan Hajnoczi <stefanha@redhat.com> | 2014-04-25 18:05:06 +0200 |
| commit | f70edf99483e7ee5f89d69fd0c51d04a35f03932 (patch) | |
| tree | 25f65cc684b71bbdc7c3ca6919049217ed86dcad | |
| parent | 92de9012904cd6ada47565f48db545d53069d933 (diff) | |
blockdev: Clean up fragile use of error_is_set()
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 internal_snapshot_prepare() 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 <armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| -rw-r--r-- | blockdev.c | 7 |
1 files changed, 5 insertions, 2 deletions
diff --git a/blockdev.c b/blockdev.c index 09826f10cf..952eb60e3d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1115,6 +1115,7 @@ typedef struct InternalSnapshotState { static void internal_snapshot_prepare(BlkTransactionState *common, Error **errp) { + Error *local_err = NULL; const char *device; const char *name; BlockDriverState *bs; @@ -1163,8 +1164,10 @@ static void internal_snapshot_prepare(BlkTransactionState *common, } /* check whether a snapshot with name exist */ - ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &old_sn, errp); - if (error_is_set(errp)) { + ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &old_sn, + &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } else if (ret) { error_setg(errp, |