[PATCH 2/4] vl: Free accel_list in configure_accelerators
On Thursday, January 9, 2020, Laurent Vivier 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
> >> ---
> >> 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-laur...@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
> Thanks,
> Laurent
>
>
>
Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
Richard Henderson 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
Reviewed-by: Alex Bennée
> ---
> 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
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
>> ---
>> 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-laur...@vivier.eu/20200108160233.991134-6-laur...@vivier.eu/
Thanks,
Laurent
Re: [PATCH 2/4] vl: Free accel_list in configure_accelerators
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
> ---
> 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
[PATCH 2/4] vl: Free accel_list in configure_accelerators
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
---
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
