On Fri, 10 Jul 2020 at 14:31, Markus Armbruster <arm...@redhat.com> wrote:
>
> The previous commit enables conversion of
>
> foo(..., &err);
> if (err) {
> ...
> }
>
> to
>
> if (!foo(..., &err)) {
> ...
> }
>
> for QemuOpts functions that now return true / false on success /
> error.
Hi; this patch changes a lot of callsites of qemu_opts_absorb_qdict()
which previously didn't check their return value to now check it,
like this:
> diff --git a/block.c b/block.c
> index 62e40db2f1..850755e04e 100644
> --- a/block.c
> +++ b/block.c
> @@ -1629,8 +1629,7 @@ static int bdrv_open_common(BlockDriverState *bs,
> BlockBackend *file,
> assert(options != NULL && bs->options != options);
>
> opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
> - qemu_opts_absorb_qdict(opts, options, &local_err);
> - if (local_err) {
> + if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> goto fail_opts;
This has triggered Coverity's "X out of Y callsites preferred to
check the return value" heuristic. Specifically,
null_file_open() (CID 1430366) and nvme_file_open() (CID 1430347)
don't check the return value. In both cases that's in some sense
OK, because they pass error_abort; but it seems a bit inconsistent
that other foo_file_open() functions check and pass up the error
and these do not.
thanks
-- PMM
