Re: [PATCH v2 3/9] tests/qtest: Allow qtest_get_machines to use an alternate QEMU binary

[Thomas Huth]

On 06/10/2023 14.39, Fabiano Rosas wrote:
> We're adding support for using more than one QEMU binary in
> tests. Modify qtest_get_machines() to take an environment variable
> that contains the QEMU binary path.
>
> Since the function keeps a cache of the machines list in the form of a
> static variable, refresh it any time the environment variable changes.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/libqtest.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 88b79cb477..47c8b6d46f 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1441,9 +1441,10 @@ struct MachInfo {
>    * Returns an array with pointers to the available machine names.
>    * The terminating entry has the name set to NULL.
>    */
> -static struct MachInfo *qtest_get_machines(void)
> +static struct MachInfo *qtest_get_machines(const char *var)
>   {
>       static struct MachInfo *machines;
> +    static char *qemu_var;
>       QDict *response, *minfo;
>       QList *list;
>       const QListEntry *p;
> @@ -1452,11 +1453,19 @@ static struct MachInfo *qtest_get_machines(void)
>       QTestState *qts;
>       int idx;
>   
> +    if (g_strcmp0(qemu_var, var)) {
> +        qemu_var = g_strdup(var);
> +
> +        /* new qemu, clear the cache */
> +        g_free(machines);
> +        machines = NULL;
> +    }
> +
>       if (machines) {
>           return machines;
>       }

After sleeping on the topic of the string handling in this patch series a 
little bit  I think it was maybe a bad idea to suggest to remove the 
g_strdups in the other patches. If you actually clear the cache here, the 
strings that previously were guaranteed to stay around until the end of the 
program might now vanish. So instead of returning the pointer to the cache 
here, it might be better to create a copy of the whole structure here and 
let the callers decide whether they want to keep it around or free it at the 
end?

 Thomas