| Message ID | 1425524720-11376-1-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
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); > } >
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); >> } >> >
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>
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); > >> } > >> > > > >
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
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
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
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 > > >
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
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); }