* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-05 3:05 [Qemu-devel] [PATCH v2] microblaze: fix memory leak arei.gonglei
@ 2015-03-17 7:15 ` Gonglei
2015-03-28 2:37 ` Gonglei
2015-03-28 2:55 ` Max Filippov
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-03-17 7:15 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Edgar E. Iglesias, Michael Tokarev
On 2015/3/5 11:05, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> When not assign a -dtb argument, the variable dtb_filename
> storage returned from qemu_find_file(), which should be freed
> after use. Alternatively we define a local variable filename,
> with 'char *' type, free after use.
>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v2: fix a complier error.
> ---
> hw/microblaze/boot.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
Ping...
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index a2843cd..352ccb1 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -113,15 +113,15 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> const char *kernel_filename;
> const char *kernel_cmdline;
> const char *dtb_arg;
> + char *filename = NULL;
>
> machine_opts = qemu_get_machine_opts();
> kernel_filename = qemu_opt_get(machine_opts, "kernel");
> kernel_cmdline = qemu_opt_get(machine_opts, "append");
> dtb_arg = qemu_opt_get(machine_opts, "dtb");
> - if (dtb_arg) { /* Preference a -dtb argument */
> - dtb_filename = dtb_arg;
> - } else { /* default to pcbios dtb as passed by machine_init */
> - dtb_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
> + /* default to pcbios dtb as passed by machine_init */
> + if (!dtb_arg) {
> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
> }
>
> boot_info.machine_cpu_reset = machine_cpu_reset;
> @@ -203,7 +203,8 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> boot_info.initrd_start,
> boot_info.initrd_end,
> kernel_cmdline,
> - dtb_filename);
> + /* Preference a -dtb argument */
> + dtb_arg ? dtb_arg : filename);
> }
> -
> + g_free(filename);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-17 7:15 ` Gonglei
@ 2015-03-28 2:37 ` Gonglei
2015-03-28 7:31 ` Edgar E. Iglesias
0 siblings, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-03-28 2:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-trivial, Edgar E. Iglesias, pbonzini, Michael Tokarev,
Peter Maydell
Hi,
Ping again... Can this patch be a raw stuff for rc2 ?
Regards,
Gonglei
On 2015/3/17 15:15, Gonglei wrote:
> On 2015/3/5 11:05, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> When not assign a -dtb argument, the variable dtb_filename
>> storage returned from qemu_find_file(), which should be freed
>> after use. Alternatively we define a local variable filename,
>> with 'char *' type, free after use.
>>
>> Cc: Michael Tokarev <mjt@tls.msk.ru>
>> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>> v2: fix a complier error.
>> ---
>> hw/microblaze/boot.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>
> Ping...
>
>> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
>> index a2843cd..352ccb1 100644
>> --- a/hw/microblaze/boot.c
>> +++ b/hw/microblaze/boot.c
>> @@ -113,15 +113,15 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
>> const char *kernel_filename;
>> const char *kernel_cmdline;
>> const char *dtb_arg;
>> + char *filename = NULL;
>>
>> machine_opts = qemu_get_machine_opts();
>> kernel_filename = qemu_opt_get(machine_opts, "kernel");
>> kernel_cmdline = qemu_opt_get(machine_opts, "append");
>> dtb_arg = qemu_opt_get(machine_opts, "dtb");
>> - if (dtb_arg) { /* Preference a -dtb argument */
>> - dtb_filename = dtb_arg;
>> - } else { /* default to pcbios dtb as passed by machine_init */
>> - dtb_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
>> + /* default to pcbios dtb as passed by machine_init */
>> + if (!dtb_arg) {
>> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
>> }
>>
>> boot_info.machine_cpu_reset = machine_cpu_reset;
>> @@ -203,7 +203,8 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
>> boot_info.initrd_start,
>> boot_info.initrd_end,
>> kernel_cmdline,
>> - dtb_filename);
>> + /* Preference a -dtb argument */
>> + dtb_arg ? dtb_arg : filename);
>> }
>> -
>> + g_free(filename);
>> }
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-28 2:37 ` Gonglei
@ 2015-03-28 7:31 ` Edgar E. Iglesias
2015-03-28 7:46 ` Gonglei
0 siblings, 1 reply; 10+ messages in thread
From: Edgar E. Iglesias @ 2015-03-28 7:31 UTC (permalink / raw)
To: Gonglei
Cc: qemu-trivial, Peter Maydell, Michael Tokarev, qemu-devel, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]
On 28/03/2015 12:38 pm, "Gonglei" <arei.gonglei@huawei.com> wrote:
>
> Hi,
>
> Ping again... Can this patch be a raw stuff for rc2 ?
Looks good to me, thanks.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Can this go through -trivial?
Cheers,
Edgar
>
> Regards,
> Gonglei
>
> On 2015/3/17 15:15, Gonglei wrote:
> > On 2015/3/5 11:05, arei.gonglei@huawei.com wrote:
> >> From: Gonglei <arei.gonglei@huawei.com>
> >>
> >> When not assign a -dtb argument, the variable dtb_filename
> >> storage returned from qemu_find_file(), which should be freed
> >> after use. Alternatively we define a local variable filename,
> >> with 'char *' type, free after use.
> >>
> >> Cc: Michael Tokarev <mjt@tls.msk.ru>
> >> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >> ---
> >> v2: fix a complier error.
> >> ---
> >> hw/microblaze/boot.c | 13 +++++++------
> >> 1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >
> > Ping...
> >
> >> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> >> index a2843cd..352ccb1 100644
> >> --- a/hw/microblaze/boot.c
> >> +++ b/hw/microblaze/boot.c
> >> @@ -113,15 +113,15 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
> >> const char *kernel_filename;
> >> const char *kernel_cmdline;
> >> const char *dtb_arg;
> >> + char *filename = NULL;
> >>
> >> machine_opts = qemu_get_machine_opts();
> >> kernel_filename = qemu_opt_get(machine_opts, "kernel");
> >> kernel_cmdline = qemu_opt_get(machine_opts, "append");
> >> dtb_arg = qemu_opt_get(machine_opts, "dtb");
> >> - if (dtb_arg) { /* Preference a -dtb argument */
> >> - dtb_filename = dtb_arg;
> >> - } else { /* default to pcbios dtb as passed by machine_init */
> >> - dtb_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
dtb_filename);
> >> + /* default to pcbios dtb as passed by machine_init */
> >> + if (!dtb_arg) {
> >> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
> >> }
> >>
> >> boot_info.machine_cpu_reset = machine_cpu_reset;
> >> @@ -203,7 +203,8 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
> >> boot_info.initrd_start,
> >> boot_info.initrd_end,
> >> kernel_cmdline,
> >> - dtb_filename);
> >> + /* Preference a -dtb argument */
> >> + dtb_arg ? dtb_arg : filename);
> >> }
> >> -
> >> + g_free(filename);
> >> }
> >>
> >
>
>
[-- Attachment #2: Type: text/html, Size: 4154 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-28 7:31 ` Edgar E. Iglesias
@ 2015-03-28 7:46 ` Gonglei
2015-03-29 17:09 ` Michael Tokarev
0 siblings, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-03-28 7:46 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: qemu-trivial, Peter Maydell, Michael Tokarev, qemu-devel, pbonzini
On 2015/3/28 15:31, Edgar E. Iglesias wrote:
> On 28/03/2015 12:38 pm, "Gonglei" <arei.gonglei@huawei.com> wrote:
>>
>> Hi,
>>
>> Ping again... Can this patch be a raw stuff for rc2 ?
>
> Looks good to me, thanks.
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
Thanks :)
> Can this go through -trivial?
>
It's ok, but I don't know if -trivial branch maintainer has a plan to send a pull request
for rc2.
Michael?
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-28 7:46 ` Gonglei
@ 2015-03-29 17:09 ` Michael Tokarev
2015-04-28 9:27 ` Gonglei
0 siblings, 1 reply; 10+ messages in thread
From: Michael Tokarev @ 2015-03-29 17:09 UTC (permalink / raw)
To: Gonglei, Edgar E. Iglesias
Cc: qemu-trivial, Peter Maydell, qemu-devel, pbonzini
28.03.2015 10:46, Gonglei wrote:
[]
>> Can this go through -trivial?
>>
> It's ok, but I don't know if -trivial branch maintainer has a plan to send a pull request
> for rc2.
I do have plan to send a pull request, but this thing is ugly. The
original code is ugly, and your patch does not make it better. I
wanted to make some changes myself but didn't find fime for that.
Ofcourse it is okay to apply your change to plug the leak, but..
oh well... :( And if we do, we'll sure forget about that completely.
Now at least you're pinging me from time to time... ;))
Thanks,
/mjt
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-29 17:09 ` Michael Tokarev
@ 2015-04-28 9:27 ` Gonglei
0 siblings, 0 replies; 10+ messages in thread
From: Gonglei @ 2015-04-28 9:27 UTC (permalink / raw)
To: Michael Tokarev, Edgar E. Iglesias
Cc: qemu-trivial, Peter Maydell, qemu-devel, pbonzini
On 2015/3/30 1:09, Michael Tokarev wrote:
> 28.03.2015 10:46, Gonglei wrote:
> []
>>> Can this go through -trivial?
>>>
>> It's ok, but I don't know if -trivial branch maintainer has a plan to send a pull request
>> for rc2.
>
> I do have plan to send a pull request, but this thing is ugly. The
> original code is ugly, and your patch does not make it better. I
> wanted to make some changes myself but didn't find fime for that.
>
> Ofcourse it is okay to apply your change to plug the leak, but..
> oh well... :( And if we do, we'll sure forget about that completely.
> Now at least you're pinging me from time to time... ;))
>
Maybe you are waiting for my ping, right? :)
Do you have any time for this leak or code optimization now?
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-05 3:05 [Qemu-devel] [PATCH v2] microblaze: fix memory leak arei.gonglei
2015-03-17 7:15 ` Gonglei
@ 2015-03-28 2:55 ` Max Filippov
2015-04-29 17:10 ` Peter Crosthwaite
2015-04-29 17:17 ` Michael Tokarev
3 siblings, 0 replies; 10+ messages in thread
From: Max Filippov @ 2015-03-28 2:55 UTC (permalink / raw)
To: arei.gonglei; +Cc: QEMU Trivial, Edgar E. Iglesias, Michael Tokarev, qemu-devel
On Thu, Mar 5, 2015 at 6:05 AM, <arei.gonglei@huawei.com> wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> When not assign a -dtb argument, the variable dtb_filename
> storage returned from qemu_find_file(), which should be freed
> after use. Alternatively we define a local variable filename,
> with 'char *' type, free after use.
>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v2: fix a complier error.
> ---
> hw/microblaze/boot.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-05 3:05 [Qemu-devel] [PATCH v2] microblaze: fix memory leak arei.gonglei
2015-03-17 7:15 ` Gonglei
2015-03-28 2:55 ` Max Filippov
@ 2015-04-29 17:10 ` Peter Crosthwaite
2015-04-29 17:17 ` Michael Tokarev
3 siblings, 0 replies; 10+ messages in thread
From: Peter Crosthwaite @ 2015-04-29 17:10 UTC (permalink / raw)
To: gonglei
Cc: qemu-trivial, Edgar E. Iglesias, Michael Tokarev,
qemu-devel@nongnu.org Developers
On Wed, Mar 4, 2015 at 7:05 PM, <arei.gonglei@huawei.com> wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> When not assign a -dtb argument, the variable dtb_filename
> storage returned from qemu_find_file(), which should be freed
> after use. Alternatively we define a local variable filename,
> with 'char *' type, free after use.
>
> Cc: Michael Tokarev <mjt@tls.msk.ru>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> v2: fix a complier error.
> ---
> hw/microblaze/boot.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index a2843cd..352ccb1 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -113,15 +113,15 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> const char *kernel_filename;
> const char *kernel_cmdline;
> const char *dtb_arg;
> + char *filename = NULL;
>
> machine_opts = qemu_get_machine_opts();
> kernel_filename = qemu_opt_get(machine_opts, "kernel");
> kernel_cmdline = qemu_opt_get(machine_opts, "append");
> dtb_arg = qemu_opt_get(machine_opts, "dtb");
> - if (dtb_arg) { /* Preference a -dtb argument */
> - dtb_filename = dtb_arg;
> - } else { /* default to pcbios dtb as passed by machine_init */
> - dtb_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
> + /* default to pcbios dtb as passed by machine_init */
> + if (!dtb_arg) {
> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
> }
>
> boot_info.machine_cpu_reset = machine_cpu_reset;
> @@ -203,7 +203,8 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
> boot_info.initrd_start,
> boot_info.initrd_end,
> kernel_cmdline,
> - dtb_filename);
> + /* Preference a -dtb argument */
> + dtb_arg ? dtb_arg : filename);
> }
> -
> + g_free(filename);
> }
> --
> 1.7.12.4
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
2015-03-05 3:05 [Qemu-devel] [PATCH v2] microblaze: fix memory leak arei.gonglei
` (2 preceding siblings ...)
2015-04-29 17:10 ` Peter Crosthwaite
@ 2015-04-29 17:17 ` Michael Tokarev
3 siblings, 0 replies; 10+ messages in thread
From: Michael Tokarev @ 2015-04-29 17:17 UTC (permalink / raw)
To: arei.gonglei, qemu-devel; +Cc: qemu-trivial, Edgar E. Iglesias
05.03.2015 06:05, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> When not assign a -dtb argument, the variable dtb_filename
> storage returned from qemu_find_file(), which should be freed
> after use. Alternatively we define a local variable filename,
> with 'char *' type, free after use.
Actually now when I look at it all again I think this is the
solution I was thinking about when saying about a "better fix".
I dunno why I haven't seen this before.
I just applied this patch to -trivial. Thank you very much for
your patience!
/mjt
^ permalink raw reply [flat|nested] 10+ messages in thread