On 05/31/2016 04:35 AM, Arnaldo Carvalho de Melo wrote:
Em Tue, May 31, 2016 at 01:44:10AM +0900, Taeung Song escreveu:
show_config() was reimplemented using perf_config()
so it isn't needed to use perf_config_set__new() at cmd_config().
And perf_config_set__delete() isn't needed at cmd_config() because of
calling the function at run_builtin() when a sub-command finished.

And it isn't also needed to declare 'config_set' as extern variable
because the variable is only handled at util/config.c from now on.

But the existing code looks more natural, i.e. before dealing with the
config_set, we try instantiating it, handling errors at constructor
calling time, etc.

Then, finally, calling the destructor when we don't need it anymore.

I understood it.
The important thing is when it is instantiated or isn't needed,
not where it is handled (i.e. at util/config.c). right ?

So we need to use the config set as extern variable
and it would be better to keep the existing code at cmd_config() as you said.

I'll send v4 with modified code soon ! :-)

Thanks,
Taeung

Cc: Namhyung Kim <namhy...@kernel.org>
Cc: Jiri Olsa <jo...@redhat.com>
Cc: Masami Hiramatsu <mhira...@kernel.org>
Cc: Alexander Shishkin <alexander.shish...@linux.intel.com>
Signed-off-by: Taeung Song <treeze.tae...@gmail.com>
---
  tools/perf/builtin-config.c | 8 --------
  tools/perf/util/config.h    | 2 --
  2 files changed, 10 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index cf6c396..412c725 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -64,12 +64,6 @@ int cmd_config(int argc, const char **argv, const char 
*prefix __maybe_unused)
        else if (use_user_config)
                config_exclusive_filename = user_config;

-       config_set = perf_config_set__new();
-       if (!config_set) {
-               ret = -1;
-               goto out_err;
-       }
-
        switch (actions) {
        case ACTION_LIST:
                if (argc) {
@@ -90,7 +84,5 @@ int cmd_config(int argc, const char **argv, const char 
*prefix __maybe_unused)
                usage_with_options(config_usage, config_options);
        }

-       perf_config_set__delete();
-out_err:
        return ret;
  }
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index e9b47b3..be4e603 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -20,8 +20,6 @@ struct perf_config_set {
        struct list_head sections;
  };

-extern struct perf_config_set *config_set;
-
  struct perf_config_set *perf_config_set__new(void);
  void perf_config_set__delete(void);

--
2.5.0

Reply via email to