On Mon, Jun 30, 2025 at 04:59:10PM -0300, Fabiano Rosas wrote:
> The documentation of qobject_from_jsonv() states that it takes
> ownership of any %p arguments passed in.
>
> Next patches will add config-passing to the tests, so take an extra
> reference in the migrate_qmp* functions to ensure the config is not
> freed from under us.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> tests/qtest/migration/migration-qmp.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/migration/migration-qmp.c
> b/tests/qtest/migration/migration-qmp.c
> index fb59741b2c..d82ac8c750 100644
> --- a/tests/qtest/migration/migration-qmp.c
> +++ b/tests/qtest/migration/migration-qmp.c
> @@ -97,7 +97,8 @@ void migrate_qmp_fail(QTestState *who, const char *uri,
> }
>
> err = qtest_qmp_assert_failure_ref(
> - who, "{ 'execute': 'migrate', 'arguments': %p}", args);
> + who, "{ 'execute': 'migrate', 'arguments': %p}",
> + qdict_clone_shallow(args));
>
> g_assert(qdict_haskey(err, "desc"));
>
> @@ -136,7 +137,8 @@ void migrate_qmp(QTestState *who, QTestState *to, const
> char *uri,
> }
>
> qtest_qmp_assert_success(who,
> - "{ 'execute': 'migrate', 'arguments': %p}",
> args);
> + "{ 'execute': 'migrate', 'arguments': %p}",
> + qdict_clone_shallow(args));
> }
>
> void migrate_set_capability(QTestState *who, const char *capability,
> @@ -174,7 +176,7 @@ void migrate_incoming_qmp(QTestState *to, const char
> *uri, QObject *channels,
> migrate_set_capability(to, "events", true);
>
> rsp = qtest_qmp(to, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
> - args);
> + qdict_clone_shallow(args));
Isn't it intentional to pass over the ownership in the three places here?
I don't see otherwise where args got freed.
OTOH, I saw there're yet another three similar usages of %p in framework.c:
x1:migration [migration-params-caps-no-config]$ git grep -A1 %p framework.c
framework.c: migrate_qmp_fail(from, args->connect_uri, NULL, "{
'config': %p }",
framework.c- args->start.config);
--
framework.c: migrate_qmp(from, to, args->connect_uri, NULL, "{ 'config': %p
}",
framework.c- args->start.config);
--
framework.c: migrate_incoming_qmp(to, args->connect_uri, NULL, "{ 'config':
%p }",
framework.c- args->start.config);
They seem to be suspecious instead, as they seem to have lost ownership of
args->start.config, so args->start.config can start to point to garbage?
>
> if (!qdict_haskey(rsp, "return")) {
> g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(rsp), true);
> --
> 2.35.3
>
--
Peter Xu
