Thomas Huth <th...@redhat.com> writes:
> 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 <faro...@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?
Hm, let me try that out. We could have a 'bool refresh' parameter in the
top level API then, which would be a clearer interface perhaps.
Thanks
