1. Patchew
  2. QEMU
  3. [PATCH 0/4] vl: Fixes for cleanups to -accel
  4. View patch

[PATCH 2/4] vl: Free accel_list in configure_accelerators

Richard Henderson posted 4 patches 6 years, 1 month ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH 2/4] vl: Free accel_list in configure_accelerators
Posted by Richard Henderson 6 years, 1 month ago 
We allocate the list with g_strsplit, so free it too.
This freeing was lost during one of the rearrangements.

Fixes: 6f6e1698a68c
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 vl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index be79b03c1a..c9329fe699 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
 static void configure_accelerators(const char *progname)
 {
     const char *accel;
-    char **accel_list, **tmp;
     bool init_failed = false;
 
     qemu_opts_foreach(qemu_find_opts("icount"),
@@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
 
     accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
     if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+        char **accel_list, **tmp;
+
         if (accel == NULL) {
             /* Select the default accelerator */
             if (!accel_find("tcg") && !accel_find("kvm")) {
@@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
                 error_report("invalid accelerator %s", *tmp);
             }
         }
+        g_strfreev(accel_list);
     } else {
         if (accel != NULL) {
             error_report("The -accel and \"-machine accel=\" options are incompatible");
-- 
2.20.1
Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
Posted by Alex Bennée 6 years, 1 month ago 
Richard Henderson <richard.henderson@linaro.org> writes:

> We allocate the list with g_strsplit, so free it too.
> This freeing was lost during one of the rearrangements.
>
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  vl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index be79b03c1a..c9329fe699 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>  static void configure_accelerators(const char *progname)
>  {
>      const char *accel;
> -    char **accel_list, **tmp;
>      bool init_failed = false;
>  
>      qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>  
>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>      if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> +        char **accel_list, **tmp;
> +
>          if (accel == NULL) {
>              /* Select the default accelerator */
>              if (!accel_find("tcg") && !accel_find("kvm")) {
> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
>                  error_report("invalid accelerator %s", *tmp);
>              }
>          }
> +        g_strfreev(accel_list);
>      } else {
>          if (accel != NULL) {
>              error_report("The -accel and \"-machine accel=\" options are incompatible");


-- 
Alex Bennée
Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
Posted by Thomas Huth 6 years, 1 month ago 
On 09/01/2020 03.17, Richard Henderson wrote:
> We allocate the list with g_strsplit, so free it too.
> This freeing was lost during one of the rearrangements.
> 
> Fixes: 6f6e1698a68c
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  vl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/vl.c b/vl.c
> index be79b03c1a..c9329fe699 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>  static void configure_accelerators(const char *progname)
>  {
>      const char *accel;
> -    char **accel_list, **tmp;
>      bool init_failed = false;
>  
>      qemu_opts_foreach(qemu_find_opts("icount"),
> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>  
>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>      if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> +        char **accel_list, **tmp;
> +
>          if (accel == NULL) {
>              /* Select the default accelerator */
>              if (!accel_find("tcg") && !accel_find("kvm")) {
> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
>                  error_report("invalid accelerator %s", *tmp);
>              }
>          }
> +        g_strfreev(accel_list);
>      } else {
>          if (accel != NULL) {
>              error_report("The -accel and \"-machine accel=\" options are incompatible");
> 

FYI, a fix for this is already part of Laurent's "Trivial branch
patches" PULL request from yesterday.

 Thomas
Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
Posted by Laurent Vivier 6 years, 1 month ago 
On 09/01/2020 09:18, Thomas Huth wrote:
> On 09/01/2020 03.17, Richard Henderson wrote:
>> We allocate the list with g_strsplit, so free it too.
>> This freeing was lost during one of the rearrangements.
>>
>> Fixes: 6f6e1698a68c
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  vl.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/vl.c b/vl.c
>> index be79b03c1a..c9329fe699 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>>  static void configure_accelerators(const char *progname)
>>  {
>>      const char *accel;
>> -    char **accel_list, **tmp;
>>      bool init_failed = false;
>>  
>>      qemu_opts_foreach(qemu_find_opts("icount"),
>> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char *progname)
>>  
>>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
>>      if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
>> +        char **accel_list, **tmp;
>> +
>>          if (accel == NULL) {
>>              /* Select the default accelerator */
>>              if (!accel_find("tcg") && !accel_find("kvm")) {
>> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char *progname)
>>                  error_report("invalid accelerator %s", *tmp);
>>              }
>>          }
>> +        g_strfreev(accel_list);
>>      } else {
>>          if (accel != NULL) {
>>              error_report("The -accel and \"-machine accel=\" options are incompatible");
>>
> 
> FYI, a fix for this is already part of Laurent's "Trivial branch
> patches" PULL request from yesterday.

https://patchew.org/QEMU/20200108160233.991134-1-laurent@vivier.eu/20200108160233.991134-6-laurent@vivier.eu/

Thanks,
Laurent
[PATCH 2/4] vl: Free accel_list in configure_accelerators
Posted by Aleksandar Markovic 6 years, 1 month ago 
On Thursday, January 9, 2020, Laurent Vivier <lvivier@redhat.com> wrote:

> On 09/01/2020 09:18, Thomas Huth wrote:
> > On 09/01/2020 03.17, Richard Henderson wrote:
> >> We allocate the list with g_strsplit, so free it too.
> >> This freeing was lost during one of the rearrangements.
> >>
> >> Fixes: 6f6e1698a68c
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>  vl.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/vl.c b/vl.c
> >> index be79b03c1a..c9329fe699 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2748,7 +2748,6 @@ static int do_configure_accelerator(void *opaque,
> QemuOpts *opts, Error **errp)
> >>  static void configure_accelerators(const char *progname)
> >>  {
> >>      const char *accel;
> >> -    char **accel_list, **tmp;
> >>      bool init_failed = false;
> >>
> >>      qemu_opts_foreach(qemu_find_opts("icount"),
> >> @@ -2756,6 +2755,8 @@ static void configure_accelerators(const char
> *progname)
> >>
> >>      accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> >>      if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
> >> +        char **accel_list, **tmp;
> >> +
> >>          if (accel == NULL) {
> >>              /* Select the default accelerator */
> >>              if (!accel_find("tcg") && !accel_find("kvm")) {
> >> @@ -2787,6 +2788,7 @@ static void configure_accelerators(const char
> *progname)
> >>                  error_report("invalid accelerator %s", *tmp);
> >>              }
> >>          }
> >> +        g_strfreev(accel_list);
> >>      } else {
> >>          if (accel != NULL) {
> >>              error_report("The -accel and \"-machine accel=\" options
> are incompatible");
> >>
> >
> > FYI, a fix for this is already part of Laurent's "Trivial branch
> > patches" PULL request from yesterday.
>
> https://patchew.org/QEMU/20200108160233.991134-1-laurent@
> vivier.eu/20200108160233.991134-6-laurent@vivier.eu/
>
>
If this (from Laurent's PR) patch is merged, Richard could transform his
patch into "vl: Fix the scope of variables accel_list and tmp in
configure_accelerator()" in v2.

Whatever happens:

Reviewed by: Aleksandar Markovic <amarkovic@wavecomp.com>


> Thanks,
> Laurent
>
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.