From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753986AbcFGHpA (ORCPT ); Tue, 7 Jun 2016 03:45:00 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33571 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753860AbcFGHo4 (ORCPT ); Tue, 7 Jun 2016 03:44:56 -0400 Subject: Re: [BUGFIX][PATCH v6 2/9] perf config: If collect_config() is failed, finally free a config set after it is done To: Arnaldo Carvalho de Melo References: <1465210380-26749-1-git-send-email-treeze.taeung@gmail.com> <1465210380-26749-3-git-send-email-treeze.taeung@gmail.com> <20160606202355.GB15108@kernel.org> <5755ED20.7080200@gmail.com> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Alexander Shishkin , Masami Hiramatsu , Jiri Olsa From: Taeung Song Message-ID: <57567B72.7090103@gmail.com> Date: Tue, 7 Jun 2016 16:44:50 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <5755ED20.7080200@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/07/2016 06:37 AM, Taeung Song wrote: > > > On 06/07/2016 05:23 AM, Arnaldo Carvalho de Melo wrote: >> Em Mon, Jun 06, 2016 at 07:52:53PM +0900, Taeung Song escreveu: >>> Because of die() at perf_parse_file() a config set was freed >>> in collect_config(), if failed. >>> But it is natural to free a config set after collect_config() is done >>> when some problems happened. >>> >>> So, in case of failure, lastly free a config set at >>> perf_config_set__new() >>> instead of freeing the config set in collect_config(). >>> >>> Cc: Namhyung Kim >>> Cc: Jiri Olsa >>> Cc: Masami Hiramatsu >>> Cc: Alexander Shishkin >>> Signed-off-by: Taeung Song >>> --- >>> tools/perf/util/config.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >>> index b500737..d013f90 100644 >>> --- a/tools/perf/util/config.c >>> +++ b/tools/perf/util/config.c >>> @@ -639,7 +639,6 @@ static int collect_config(const char *var, const >>> char *value, >>> >>> out_free: >>> free(key); >>> - perf_config_set__delete(set); >>> return -1; >>> } >>> >>> @@ -649,7 +648,8 @@ struct perf_config_set *perf_config_set__new(void) >>> >>> if (set) { >>> INIT_LIST_HEAD(&set->sections); >>> - perf_config(collect_config, set); >>> + if (perf_config(collect_config, set) < 0) >>> + perf_config_set__delete(set); >>> } >>> >>> return set; >> >> You can't do that, there is something missing, without looking at the >> code I think you need: >> >> if (set) { >> INIT_LIST_HEAD(&set->sections); >> - perf_config(collect_config, set); >> + if (perf_config(collect_config, set) < 0) { >> + perf_config_set__delete(set); >> + set = NULL; >> + } >> } >> >> return set; >> >> No? >> > > Granted > Sorry for missing above.. > > I modified using 'return NULL;' instead of 'set = NULL;' as below > > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > index c73f1c4..cb749d3 100644 > --- a/tools/perf/util/config.c > +++ b/tools/perf/util/config.c > @@ -643,7 +643,6 @@ static int collect_config(const char *var, const > char *value, > > out_free: > free(key); > - perf_config_set__delete(set); > return -1; > } > > @@ -653,7 +652,10 @@ struct perf_config_set *perf_config_set__new(void) > > if (set) { > INIT_LIST_HEAD(&set->sections); > - perf_config(collect_config, set); > + if (perf_config(collect_config, set) < 0) { > + perf_config_set__delete(set); > + return NULL; > + } > } > > return set; > > Because in near future, perf_config_set__delete() will use zfree(). > > will send changed this patch soon ! > Thank you for your review :) > Hum.. my answer was stupid. There isn't difference between 'return NULL;' and 'set = NULL;' as a result at perf_config_set__new(). And zfree() at perf_config_set__delete() aren't related to this situation.. Anyway.. I'll send v7 with changed this patch as you said!! Thanks, Taeung