Re: [PATCH v5 1/4] perf config: Introduce perf_config_set class

[Taeung Song]

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.tae...@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