From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932614AbcGDTJw (ORCPT ); Mon, 4 Jul 2016 15:09:52 -0400 Received: from mail.kernel.org ([198.145.29.136]:51912 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932576AbcGDTJv (ORCPT ); Mon, 4 Jul 2016 15:09:51 -0400 Date: Mon, 4 Jul 2016 16:09:47 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: lkml , David Ahern , Ingo Molnar , Namhyung Kim , Peter Zijlstra Subject: Re: [PATCH 2/4] perf tools: Do the error path hist_entry release in hist_entry__new Message-ID: <20160704190947.GW5324@kernel.org> References: <1467640899-3776-1-git-send-email-jolsa@kernel.org> <1467640899-3776-3-git-send-email-jolsa@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467640899-3776-3-git-send-email-jolsa@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Mon, Jul 04, 2016 at 04:01:37PM +0200, Jiri Olsa escreveu: > It's better to release the hist_entry object in > hist_entry__new function (where it's allocated) > rather than in hist_entry__init. Please combine this with the previous patch, that way this all gets clearer :-\ - Arnaldo > Link: http://lkml.kernel.org/n/tip-uoatzgsbdk3ebaeu56kdb9ku@git.kernel.org > Signed-off-by: Jiri Olsa > --- > tools/perf/util/hist.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index ed7bea9aa68d..04f3b52a319c 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -360,10 +360,8 @@ static int hist_entry__init(struct hist_entry *he, > > if (symbol_conf.cumulate_callchain) { > he->stat_acc = malloc(sizeof(he->stat)); > - if (he->stat_acc == NULL) { > - free(he); > + if (he->stat_acc == NULL) > return -ENOMEM; > - } > memcpy(he->stat_acc, &he->stat, sizeof(he->stat)); > if (!sample_self) > memset(&he->stat, 0, sizeof(he->stat)); > @@ -381,7 +379,6 @@ static int hist_entry__init(struct hist_entry *he, > if (he->branch_info == NULL) { > map__zput(he->ms.map); > free(he->stat_acc); > - free(he); > return -ENOMEM; > } > > @@ -415,7 +412,6 @@ static int hist_entry__init(struct hist_entry *he, > map__put(he->mem_info->daddr.map); > } > free(he->stat_acc); > - free(he); > return -ENOMEM; > } > } > @@ -439,10 +435,13 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template, > callchain_size = sizeof(struct callchain_root); > > he = zalloc(sizeof(*he) + callchain_size); > - if (he) > + if (he) { > err = hist_entry__init(he, template, sample_self); > + if (err) > + zfree(&he); > + } > > - return err ? NULL : he; > + return he; > } > > static u8 symbol__parent_filter(const struct symbol *parent) > -- > 2.4.11