| Message ID | 20230626215926.2522656-9-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | maintainer omnibus: testing, fuzz, plugins, documentation | expand |
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), .description = "Predefined generic-fuzz config.", .get_init_cmdline = generic_fuzz_predefined_config_cmdline, .pre_fuzz = generic_pre_fuzz,
On 230626 2259, Alex Bennée 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> Reviewed-by: Alexander Bulekov <alxndr@bu.edu> Thank you > --- > 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, > -- > 2.39.2 >
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.", ---
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.
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,
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(-)