Subject: Re: [PATCH v5 1/4] perf config: Introduce perf_config_set class
To: Masami Hiramatsu <mhiramat@kernel.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>
References: <1459616904-1682-1-git-send-email-treeze.taeung@gmail.com>
 <1459616904-1682-2-git-send-email-treeze.taeung@gmail.com>
 <CADyE4xGtr-54PThpkwgPUiSaxT1FtfLBRjAAsLkYEwz6P8783g@mail.gmail.com>
Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
From: Taeung Song <treeze.taeung@gmail.com>
Message-ID: <570228AC.8090004@gmail.com>
Date: Mon, 4 Apr 2016 17:41:16 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.6.0
MIME-Version: 1.0
In-Reply-To: <CADyE4xGtr-54PThpkwgPUiSaxT1FtfLBRjAAsLkYEwz6P8783g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2228
Lines: 78

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@gmail.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(&section->items);
>> +       section->name = strdup(section_name);
>> +       if (!section->name) {
>> +               pr_debug("%s: strdup failed\n", __func__);
>> +               free(section);
>> +               return NULL;
>> +       }
>> +
>> +       list_add_tail(&section->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, &section->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