From 0fb6395c0cb5046432a80d608ddde7a3b2f8a9ae Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 16:50:31 +0200 Subject: Use error_is_set() only when necessary (again) error_is_set(&var) is the same as var != NULL, but it takes whole-program analysis to figure that out. Unnecessarily hard for optimizers, static checkers, and human readers. Commit 84d18f0 dumbed it down to obvious, but a few more have crept in since, and documentation was overlooked. Dumb these down, too. Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- block/mirror.c | 2 +- block/nfs.c | 2 +- block/quorum.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'block') diff --git a/block/mirror.c b/block/mirror.c index 2618c3763c..36f4f8e8bd 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -680,7 +680,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, mirror_start_job(bs, base, speed, 0, 0, on_error, on_error, cb, opaque, &local_err, &commit_active_job_driver, false, base); - if (error_is_set(&local_err)) { + if (local_err) { error_propagate(errp, local_err); goto error_restore_flags; } diff --git a/block/nfs.c b/block/nfs.c index 98aa363e48..9fa831f160 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -343,7 +343,7 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags, opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); - if (error_is_set(&local_err)) { + if (local_err) { error_propagate(errp, local_err); return -EINVAL; } diff --git a/block/quorum.c b/block/quorum.c index 7f580a83b5..ecec3a5407 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -753,7 +753,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, opts = qemu_opts_create(&quorum_runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); - if (error_is_set(&local_err)) { + if (local_err) { ret = -EINVAL; goto exit; } @@ -828,7 +828,7 @@ close_exit: g_free(opened); exit: /* propagate error */ - if (error_is_set(&local_err)) { + if (local_err) { error_propagate(errp, local_err); } QDECREF(list); -- cgit v1.2.3-55-g7522 From 92de9012904cd6ada47565f48db545d53069d933 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 16:50:33 +0200 Subject: nbd: Use return values instead of error_is_set(errp) Using error_is_set(errp) to check whether a function call failed is fragile: it breaks when errp is null. Check perfectly suitable return values instead when possible. errp can't be null there now, but this is more robust and more obviously correct Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/nbd.c b/block/nbd.c index 55124239df..613f2581ae 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -175,7 +175,7 @@ static void nbd_parse_filename(const char *filename, QDict *options, InetSocketAddress *addr = NULL; addr = inet_parse(host_spec, errp); - if (error_is_set(errp)) { + if (!addr) { goto out; } -- cgit v1.2.3-55-g7522 From 172fc4dd33e604dcf868c28e73398c19e161708b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 16:50:35 +0200 Subject: iscsi: Don't use error_is_set() to suppress additional errors Using error_is_set(errp) that way can sweep programming errors under the carpet when we get called incorrectly with an error set. Commit 24d3bd6 added a broken error path to iscsi_do_inquiry(): it first calls error_setg(), then jumps to the preexisting error label, where error_setg() gets called again, triggering an assertion failure. Commit cbee81f fixed this by guarding the second error_setg() with an error_is_set(). Replace this fix by a simpler and safer one: jump right behind the second error_setg(). Signed-off-by: Markus Armbruster Reviewed-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/iscsi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index a636ea4f53..a30202b4fe 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1095,16 +1095,15 @@ static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, *inq = scsi_datain_unmarshall(task); if (*inq == NULL) { error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); - goto fail; + goto fail_with_err; } return task; fail: - if (!error_is_set(errp)) { - error_setg(errp, "iSCSI: Inquiry command failed : %s", - iscsi_get_error(iscsi)); - } + error_setg(errp, "iSCSI: Inquiry command failed : %s", + iscsi_get_error(iscsi)); +fail_with_err: if (task != NULL) { scsi_free_scsi_task(task); } -- cgit v1.2.3-55-g7522