From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46958) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2CZY-0006BP-H5 for qemu-devel@nongnu.org; Fri, 27 Nov 2015 01:32:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2CZU-0002pm-LC for qemu-devel@nongnu.org; Fri, 27 Nov 2015 01:31:56 -0500 Received: from smtpcm9-314.freemail.mail.aliyun.com ([110.75.46.14]:44302) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2CZT-0002me-76 for qemu-devel@nongnu.org; Fri, 27 Nov 2015 01:31:52 -0500 References: <1448539212-7080-1-git-send-email-opensource.dxs@aliyun.com> <87lh9l3ppu.fsf@blackfin.pond.sub.org> From: Stefano Dong Message-ID: <5657F8C7.3060503@aliyun.com> Date: Fri, 27 Nov 2015 14:31:35 +0800 MIME-Version: 1.0 In-Reply-To: <87lh9l3ppu.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] Fix memory leak on error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: zhongguocheng@huawei.com, agraf@suse.de, zhugh.fnst@cn.fujitsu.com, qemu-devel@nongnu.org, mst@redhat.com On 2015/11/26 20:33, Markus Armbruster wrote: > Stefano Dong (董兴水) writes: > >> hw/ppc/spapr.c: Fix memory leak on error, it was introduced in bc09e0611 >> hw/acpi/memory_hotplug.c: Fix memory leak on error, it was introduced in 34f2af3d >> >> Signed-off-by: Stefano Dong (董兴水) >> --- >> hw/acpi/memory_hotplug.c | 1 + >> hw/ppc/spapr.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >> index ce428df..e4b9a01 100644 >> --- a/hw/acpi/memory_hotplug.c >> +++ b/hw/acpi/memory_hotplug.c >> @@ -155,6 +155,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data, >> qapi_event_send_mem_unplug_error(dev->id, >> error_get_pretty(local_err), >> &error_abort); >> + error_free(local_err); >> break; >> } >> trace_mhp_acpi_pc_dimm_deleted(mem_st->selector); >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 030ee35..3bb8bcd 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -125,6 +125,7 @@ static XICSState *xics_system_init(MachineState *machine, >> error_report("kernel_irqchip requested but unavailable: %s", >> error_get_pretty(err)); >> } >> + error_free(err); >> } >> >> if (!icp) { > > Two independent instances of the same kind of bug (failure to free an > Error after handling it). Collecting multiple fixes of the same kind in > one patch can be fine. Note, however, that the combined patch spans > separately maintained areas of the code: > > $ scripts/get_maintainer.pl -f hw/acpi/memory_hotplug.c > "Michael S. Tsirkin" (supporter:ACPI/SMBIOS) > Igor Mammedov (supporter:ACPI/SMBIOS) > $ scripts/get_maintainer.pl -f hw/ppc/spapr.c > David Gibson (supporter:sPAPR (pseries)) > Alexander Graf (supporter:sPAPR (pseries)) > Eduardo Habkost (maintainer:NUMA) > qemu-ppc@nongnu.org (open list:sPAPR (pseries)) > > If the combined patch is trivial, cc: qemu-trivial@nongnu.org. I figure > this one could qualify. > > If it isn't, split it up along mainenance boundaries, and cc: the > maintainer(s) for each part. > > When you fix something with serious impact, the commit message should > give a clue when it was broken. For minor bugs like these, it's still > nice to do. The acpi_memory_hotplug_write() leak was introduced in > bc09e06 (v2.4.0), and the xics_system_init() leak in commit 34f2af3 > (v2.3.0). Thanks for fixing the mess I made there, by the way. > > Since this is a straightforward bug fix, proposing it for inclusion into > 2.5 makes sense. You can do that by putting [PATCH for-2.5] into the > subject. Thank you Markus! Thanks for teaching me a lot! You are so kind and friendly! >