Re: [PATCH v2 08/26] tests/qtests: clean-up and fix leak in generic_fuzz

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 27/6/23 09:43, Juan Quintela wrote:
>> Alex Bennée <alex.bennee@linaro.org> wrote:
>>> An update to the clang tooling detects more issues with the code
>>> including a memory leak from the g_string_new() allocation. Clean up
>>> the code with g_autoptr and use ARRAY_SIZE while we are at it.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>   tests/qtest/fuzz/generic_fuzz.c | 11 ++++-------
>>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/qtest/fuzz/generic_fuzz.c 
>>> b/tests/qtest/fuzz/generic_fuzz.c
>>> index c525d22951..a4841181cc 100644
>>> --- a/tests/qtest/fuzz/generic_fuzz.c
>>> +++ b/tests/qtest/fuzz/generic_fuzz.c
>>> @@ -954,17 +954,14 @@ static void register_generic_fuzz_targets(void)
>>>               .crossover = generic_fuzz_crossover
>>>       });
>>>   -    GString *name;
>>> +    g_autoptr(GString) name = g_string_new("");
>>>       const generic_fuzz_config *config;
>>>   -    for (int i = 0;
>>> -         i < sizeof(predefined_configs) / sizeof(generic_fuzz_config);
>>> -         i++) {
>>> +    for (int i = 0; i < ARRAY_SIZE(predefined_configs); i++) {
>>>           config = predefined_configs + i;
>>> -        name = g_string_new("generic-fuzz");
>>> -        g_string_append_printf(name, "-%s", config->name);
>>> +        g_string_printf(name, "generic-fuzz-%s", config->name);
>>>           fuzz_add_target(&(FuzzTarget){
>>> -                .name = name->str,
>>> +                .name = g_strdup(name->str),
>>>                   .description = "Predefined generic-fuzz config.",
>>>                   .get_init_cmdline = 
>>> generic_fuzz_predefined_config_cmdline,
>>>                   .pre_fuzz = generic_pre_fuzz,
>> Once that you are here, what about?
>> (Yes, I didn't care about the ARRAY_SIZE) but you got the idea.
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> To your proposal with/without the change that I proposse.
>> modified   tests/qtest/fuzz/generic_fuzz.c
>> @@ -954,17 +954,14 @@ static void register_generic_fuzz_targets(void)
>>               .crossover = generic_fuzz_crossover
>>       });
>>   -    GString *name;
>>       const generic_fuzz_config *config;
>>         for (int i = 0;
>>            i < sizeof(predefined_configs) / sizeof(generic_fuzz_config);
>>            i++) {
>>           config = predefined_configs + i;
>> -        name = g_string_new("generic-fuzz");
>> -        g_string_append_printf(name, "-%s", config->name);
>>           fuzz_add_target(&(FuzzTarget){
>> -                .name = name->str,
>> +                .name = g_strdup_printf("generic-fuzz-%s", config->name),
>
> Even simpler is g_strconcat() suggested by Richard in v1:
> https://lore.kernel.org/qemu-devel/42b497a0-e234-64db-e845-1c37b67839fc@linaro.org/

Doh - missed that will fix up.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro