On 01/30/2014 07:07 AM, Markus Armbruster wrote: > 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. Dumb it down to > obvious. > > Gets rid of several dozen Coverity false positives. > > Note that the obvious form is already used in many places. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > ---
> 37 files changed, 156 insertions(+), 156 deletions(-)
Good diffstat - shows it should be fairly mechanical.
> @@ -1399,7 +1399,7 @@ fail:
> QDECREF(bs->options);
> QDECREF(options);
> bs->options = NULL;
> - if (error_is_set(&local_err)) {
> + if (local_err) {
> error_propagate(errp, local_err);
> }
> return ret;
Is it worth a further cleanup on instances like this? That is,
error_propagate(errp, NULL) is a safe no-op, so we can avoid the 'if
(local_err)' conditional. But that should not be in this patch (keep
the mechanical changes easy).
> +++ b/block/snapshot.c
> @@ -345,7 +345,7 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState
> *bs,
> ret = bdrv_snapshot_load_tmp(bs, NULL, id_or_name, &local_err);
> }
>
> - if (error_is_set(&local_err)) {
> + if (local_err) {
> error_propagate(errp, local_err);
> }
Another example that can be simplified.
> +++ b/tests/test-qmp-input-strict.c
> @@ -92,7 +92,7 @@ static void test_validate_struct(TestInputVisitorData *data,
> v = validate_test_init(data, "{ 'integer': -42, 'boolean': true,
> 'string': 'foo' }");
>
> visit_type_TestStruct(v, &p, NULL, &errp);
> - g_assert(!error_is_set(&errp));
> + g_assert(!errp);
This (and other places in test files) chould use
visit_type_TestStruct(v, &p, NULL, &error_abort) and ditch local errp.
But that's a separate patch as well.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
