| Message ID | 55670122.50101@msgid.tls.msk.ru |
|---|---|
| State | New |
| Headers | show |
On 2015/5/28 19:50, Michael Tokarev wrote: > 28.05.2015 14:13, Shannon Zhao пишет: >> From: Shannon Zhao <shannon.zhao@linaro.org> >> >> valgrind complains about: >> ==7055== 58 bytes in 1 blocks are definitely lost in loss record 1,471 of 2,192 >> ==7055== at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) >> ==7055== by 0x24410F: malloc_and_trace (vl.c:2556) >> ==7055== by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) >> ==7055== by 0x64DEFD7: g_strndup (in /usr/lib64/libglib-2.0.so.0.3600.3) >> ==7055== by 0x650181A: g_vasprintf (in /usr/lib64/libglib-2.0.so.0.3600.3) >> ==7055== by 0x64DF0CC: g_strdup_vprintf (in /usr/lib64/libglib-2.0.so.0.3600.3) >> ==7055== by 0x64DF188: g_strdup_printf (in /usr/lib64/libglib-2.0.so.0.3600.3) >> ==7055== by 0x242F81: qemu_find_file (vl.c:2121) >> ==7055== by 0x217A32: clipper_init (dp264.c:105) >> ==7055== by 0x2484DA: main (vl.c:4249) >> >> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >> --- >> hw/alpha/dp264.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c >> index 9fe7e8b..0c5395f 100644 >> --- a/hw/alpha/dp264.c >> +++ b/hw/alpha/dp264.c >> @@ -56,6 +56,7 @@ static void clipper_init(MachineState *machine) >> qemu_irq rtc_irq; >> long size, i; >> const char *palcode_filename; >> + char *filename; >> uint64_t palcode_entry, palcode_low, palcode_high; >> uint64_t kernel_entry, kernel_low, kernel_high; >> >> @@ -102,18 +103,20 @@ static void clipper_init(MachineState *machine) >> but one explicitly written for the emulation, we might as >> well load it directly from and ELF image. */ >> palcode_filename = (bios_name ? bios_name : "palcode-clipper"); >> - palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, palcode_filename); >> - if (palcode_filename == NULL) { >> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, palcode_filename); >> + if (filename == NULL) { >> hw_error("no palcode provided\n"); >> exit(1); >> } >> - size = load_elf(palcode_filename, cpu_alpha_superpage_to_phys, >> + size = load_elf(filename, cpu_alpha_superpage_to_phys, >> NULL, &palcode_entry, &palcode_low, &palcode_high, >> 0, EM_ALPHA, 0); >> if (size < 0) { >> - hw_error("could not load palcode '%s'\n", palcode_filename); >> + hw_error("could not load palcode '%s'\n", filename); >> + g_free(filename); > > In a previous patch you _removed_ g_free() before exit(), here' you're > adding one. I don't think there's any need to free things before exit(), > but at least we should be consistent, maybe at least within one series :) > >> exit(1); >> } >> + g_free(filename); > > How about this: > > diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c > index 9fe7e8b..f86e7bb 100644 > --- a/hw/alpha/dp264.c > +++ b/hw/alpha/dp264.c > @@ -55,7 +55,7 @@ static void clipper_init(MachineState *machine) > ISABus *isa_bus; > qemu_irq rtc_irq; > long size, i; > - const char *palcode_filename; > + char *palcode_filename; > uint64_t palcode_entry, palcode_low, palcode_high; > uint64_t kernel_entry, kernel_low, kernel_high; > > @@ -101,8 +101,8 @@ static void clipper_init(MachineState *machine) > /* Load PALcode. Given that this is not "real" cpu palcode, > but one explicitly written for the emulation, we might as > well load it directly from and ELF image. */ > - palcode_filename = (bios_name ? bios_name : "palcode-clipper"); > - palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, palcode_filename); > + palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, > + bios_name ? bios_name : "palcode-clipper"); > if (palcode_filename == NULL) { > hw_error("no palcode provided\n"); > exit(1); > @@ -114,6 +114,7 @@ static void clipper_init(MachineState *machine) > hw_error("could not load palcode '%s'\n", palcode_filename); > exit(1); > } > + g_free(palcode_filename); > > /* Start all cpus at the PALcode RESET entry point. */ > for (i = 0; i < smp_cpus; ++i) { > > > I'm not saying it is better, it is just that we don't really > need the original basename of the file and hence don't need > second variable. > Yes, I've thought about this way but it's gone. > If you like it I'd apply it, of I'll apply your version > (without g_free() before exit). > I'm good with your version. Thanks,
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index 9fe7e8b..f86e7bb 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -55,7 +55,7 @@ static void clipper_init(MachineState *machine) ISABus *isa_bus; qemu_irq rtc_irq; long size, i; - const char *palcode_filename; + char *palcode_filename; uint64_t palcode_entry, palcode_low, palcode_high; uint64_t kernel_entry, kernel_low, kernel_high; @@ -101,8 +101,8 @@ static void clipper_init(MachineState *machine) /* Load PALcode. Given that this is not "real" cpu palcode, but one explicitly written for the emulation, we might as well load it directly from and ELF image. */ - palcode_filename = (bios_name ? bios_name : "palcode-clipper"); - palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, palcode_filename); + palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, + bios_name ? bios_name : "palcode-clipper"); if (palcode_filename == NULL) { hw_error("no palcode provided\n"); exit(1); @@ -114,6 +114,7 @@ static void clipper_init(MachineState *machine) hw_error("could not load palcode '%s'\n", palcode_filename); exit(1); } + g_free(palcode_filename); /* Start all cpus at the PALcode RESET entry point. */ for (i = 0; i < smp_cpus; ++i) {