[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Mon, 4 Apr 2016 17:41:16 +0900
From: Taeung Song <treeze.taeung@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v5 1/4] perf config: Introduce perf_config_set class
Hi, Masami Hiramatsu
On 04/04/2016 05:13 PM, Masami Hiramatsu wrote:
> Hi,
>
> 2016-04-03 2:08 GMT+09:00 Taeung Song <treeze.taeung@...il.com>:
> [...]
>> +static struct perf_config_section *add_section(struct list_head *sections,
>> + const char *section_name)
>> +{
>> + struct perf_config_section *section = zalloc(sizeof(*section));
>> +
>> + if (!section)
>> + return NULL;
>> +
>> + INIT_LIST_HEAD(§ion->items);
>> + section->name = strdup(section_name);
>> + if (!section->name) {
>> + pr_debug("%s: strdup failed\n", __func__);
>> + free(section);
>> + return NULL;
>> + }
>> +
>> + list_add_tail(§ion->node, sections);
>> + return section;
>> +}
>> +
>> +static struct perf_config_item *add_config_item(struct perf_config_section *section,
>> + const char *name)
>> +{
>> + struct perf_config_item *item = zalloc(sizeof(*item));
>> +
>> + if (!item)
>> + return NULL;
>> +
>> + item->name = strdup(name);
>> + if (!item->name) {
>> + pr_debug("%s: strdup failed\n", __func__);
>> + goto out_err;
>
> As you did in above add_section(), you can do free(item)
> and return NULL here instead of using goto.
>
> Other parts looks OK for me.
>
I got it.
What you mean is like below ?
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index e636d09..53e756e 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -580,15 +580,12 @@ static struct perf_config_item
*add_config_item(struct perf_config_section *sect
item->name = strdup(name);
if (!item->name) {
pr_debug("%s: strdup failed\n", __func__);
- goto out_err;
+ free(item);
+ return NULL;
}
list_add_tail(&item->node, §ion->items);
return item;
-
-out_err:
- free(item);
- return NULL;
}
static int set_value(struct perf_config_item *item, const char *value)
I'll resend v6 with this.
Thank you for your review. :-)
Taeung
Powered by blists - more mailing lists