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

[Philippe Mathieu-Daudé]

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/

-- >8 --
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -956,13 +956,8 @@ static void register_generic_fuzz_targets(void)

-    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);
+    for (int i = 0; i < ARRAY_SIZE(predefined_configs); i++) {
+        config = &predefined_configs[i];
         fuzz_add_target(&(FuzzTarget){
-                .name = name->str,
+                .name = g_strconcat("generic-fuzz-", config->name, NULL),
                 .description = "Predefined generic-fuzz config.",
---