On Thu, Sep 15, 2022 at 10:42:55PM +0200, Markus Armbruster wrote:
> The has_FOO for pointer-valued FOO are redundant, except for arrays.
> They are also a nuisance to work with. Recent commit "qapi: Start to
> elide redundant has_FOO in generated C" provided the means to elide
> them step by step. This is the step for
> tests/qapi-schema/qapi-schema-test.json.
>
> Said commit explains the transformation in more detail. The invariant
> violations mentioned there do not occur here.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> tests/qtest/qmp-cmd-test.c | 2 +-
> tests/unit/test-qmp-cmds.c | 26 +++++++++++-------------
> tests/unit/test-qmp-event.c | 4 ++--
> tests/unit/test-qobject-input-visitor.c | 2 +-
> tests/unit/test-qobject-output-visitor.c | 2 --
> tests/unit/test-visitor-serialization.c | 3 +--
> scripts/qapi/schema.py | 4 +---
> 7 files changed, 18 insertions(+), 25 deletions(-)
> diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
> index 6085c09995..2373cd64cb 100644
> --- a/tests/unit/test-qmp-cmds.c
> +++ b/tests/unit/test-qmp-cmds.c
> @@ -43,15 +43,15 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
> {
> }
>
> -FeatureStruct1 *qmp_test_features0(bool has_fs0, FeatureStruct0 *fs0,
> - bool has_fs1, FeatureStruct1 *fs1,
> - bool has_fs2, FeatureStruct2 *fs2,
> - bool has_fs3, FeatureStruct3 *fs3,
> - bool has_fs4, FeatureStruct4 *fs4,
> - bool has_cfs1, CondFeatureStruct1 *cfs1,
> - bool has_cfs2, CondFeatureStruct2 *cfs2,
> - bool has_cfs3, CondFeatureStruct3 *cfs3,
> - bool has_cfs4, CondFeatureStruct4 *cfs4,
> +FeatureStruct1 *qmp_test_features0(FeatureStruct0 *fs0,
> + FeatureStruct1 *fs1,
> + FeatureStruct2 *fs2,
> + FeatureStruct3 *fs3,
> + FeatureStruct4 *fs4,
> + CondFeatureStruct1 *cfs1,
> + CondFeatureStruct2 *cfs2,
> + CondFeatureStruct3 *cfs3,
> + CondFeatureStruct4 *cfs4,
> Error **errp)
It makes me so happy to see this change finally arrive, the new
method signature just "feels right".
> @@ -87,8 +86,8 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
>
> ud1c->string = strdup(ud1a->string);
> ud1c->integer = ud1a->integer;
> - ud1d->string = strdup(has_udb1 ? ud1b->string : "blah0");
> - ud1d->integer = has_udb1 ? ud1b->integer : 0;
> + ud1d->string = strdup(ud1b ? ud1b->string : "blah0");
> + ud1d->integer = ud1b ? ud1b->integer : 0;
Pre-existing problem. use of 'strdup' will leave these fields
NULL on OOM and nothing is checking this. Fortunately it is only
the test suite so not a big deal, but worth changing to g_strdup
at some point (separate from this patch / series). Repeated in
other chunks of the test beyond this quoted one, but I won't
point them out.
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
