perf config: Reimplement perf_config() using perf_config_set__inter()
v6 cover perf config: Reimplement perf_config() using perf_config_set__inter()
Hello, :)
This patchset is to reimplement perf_config() for efficient config management.
Everytime perf_config() is called, perf_config() always read config files. (i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')
But we need to use 'struct perf_config_set config_set' variable that already contains all config key-value pairs
to avoid this repetitive work in perf_config().
In other words, if new perf_config() is called,
only first time 'config_set' is initialized
collecting all configs from config files and it work with perf_config_set__iter().
If we do, 'config_set' can be reused wherever using perf_config() and a feature of old perf_config() is the same as new perf_config() work without the repetitive work that read the config files.
IMHO, I think this patchset is needed because not only the repetitive work should be avoided but also in near future, it would be smooth to manage perf configs.
Most important patch of this patchset is "[PATCH v5 7/9] perf config: Reimplement perf_config() using perf_config_set__iter()" and PATCH 1/9 ~ 6/9 are preparation for it.
If you give me any feedback, I'd apprecicated it. :)
Thanks,
Taeung
v6:
- add printing error message when perf_config_set__iter() is failed
- modify commit messages for bugfix 1~3 (PATCH 1/9 ~ 3/9) to help reviewers easily understand why them is needed
v5:
- solve the leak when perf_config_set__init() failed (Arnaldo)
(to clear the problem it is needed to apply the bottom bugfix 1~3 patches)
- bugfix 1) fix the problem of abnormal terminaltion at perf_parse_file() called by perf_config()
- bugfix 2) if failed at collect_config(), finally free a config set
after it is done instead of freeing the config set in the function
- bugfix 3) handle NULL pointer exception of 'set' at collect_config()
v4:
- Keep perf_config_set__delete() as it is (Arnaldo)
- Remove perf_config_set__check() (Arnaldo)
- Keep the existing code about the config set at cmd_config() (Arnaldo)
v3:
- add freeing config set after sub-command work at run_builtin() (Namhyung)
- remove needless code about the config set at cmd_config()
- add a patch about a global variable 'config_set'
v2:
- split a patch into several patches
- reimplement show_config() using new perf_config()
- modify perf_config_set__delete using global variable 'config_set'
- reset config set when only 'config' sub-commaned work because of options for config file location
Taeung Song (9):
perf config: Fix the problem of abnormal termination at
perf_parse_file()
perf config: If collect_config() is failed, finally free a config set
after it is done
perf config: Handle the error when config set is NULL at
collect_config()
perf config: Use new perf_config_set__init() to initialize config set perf config: Add global variable 'config_set'
perf config: Use zfree() instead of free() at
perf_config_set__delete()
perf config: Reimplement perf_config() using perf_config_set__iter() perf config: Reset the config set at only 'config' sub-command perf config: Reimplement show_config() using perf_config()
tools/perf/builtin-config.c | 41 +++++------- tools/perf/perf.c | 1 + tools/perf/util/cache.h | 1 + tools/perf/util/config.c | 159 +++++++++++++++++++++++++++++--------------- tools/perf/util/config.h | 4 +- 5 files changed, 124 insertions(+), 82 deletions(-)
Signature · 1 line
2.5.0
PATCH v6 1/9 perf config: Fix the problem of abnormal termination at perf_parse_file()
If a config file has wrong key-value pairs, perf process will be forcibly terminated by die() at perf_parse_file() called by perf_config() so terminal setting can be crushed because of unusual termination.
For example,
If user config file has a wrong value 'red;default' instead of a normal value like 'red, default' for a key 'colors.top',
# cat ~/.perfconfig
[colors]
medium = red;default # wrong valueand if running sub-command 'top',
# perf top
perf process is dead by force and terminal setting is broken with a messge like below.
Fatal: bad config file line 2 in /root/.perfconfig
So fix it.
If perf_config() can return on failure without calling die() at perf_parse_file(), this problem can be solved.
And if a config file has wrong values, show the error message and then use default config values instead of wrong config values.
--- tools/perf/util/config.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
tools/perf/util/config.c +7 -9
@@ -275,7 +275,8 @@ static int perf_parse_file(config_fn_t fn, void *data)break ; } } -die ("bad config file line %d in %s" , config_linenr, config_file_name); +pr_err ("bad config file line %d in %s\n" , config_linenr, config_file_name); +return -1 ; }static int parse_unit_factor (const char *end,unsigned long *val)
@@ -479,16 +480,15 @@ static int perf_config_global(void)int perf_config (config_fn_t fn,void *data) { -int ret =0 , found =0 ; +int ret =-1 ;const char *home =NULL ;/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */ if (config_exclusive_filename)return perf_config_from_file (fn, config_exclusive_filename, data);if (perf_config_system () && !access (perf_etc_perfconfig (),R_OK )) { - ret +=perf_config_from_file (fn,perf_etc_perfconfig (), - data); - found +=1 ; +if (perf_config_from_file (fn,perf_etc_perfconfig (), data) <0 ) + goto out; } home =getenv ("HOME" );
@@ -514,14 +514,12 @@ int perf_config(config_fn_t fn, void *data)if (!st.st_size) goto out_free; - ret +=perf_config_from_file (fn, user_config, data); - found +=1 ; + ret =perf_config_from_file (fn, user_config, data); + out_free:free (user_config); } out: -if (found ==0 ) -return -1 ;return ret; }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerRe: [BUGFIX][PATCH v6 1/9] perf config: Fix the problem of abnormal termination at perf_parse_file()
Em Mon, Jun 06, 2016 at 07:52:52PM +0900, Taeung Song escreveu:
Quoted 9 lines
For example,
If user config file has a wrong value 'red;default' instead of a normal value like 'red, default' for a key 'colors.top',
# cat ~/.perfconfig [colors] medium = red;default # wrong value
Quoted 8 lines
and if running sub-command 'top',
# perf topperf process is dead by force and terminal setting is broken with a messge like below.
Fatal: bad config file line 2 in
/root/.perfconfig
Thanks, reproduced, tested, applied.
- Arnaldo
tip-bot for Taeung Song
merge bot[tip:perf/core] perf config: Fix abnormal termination at perf_parse_file()
In reply to Taeung Songperf config: Fix abnormal termination at perf_parse_file()
Commit-ID: 78f71c996fb92d129ec75d572e2f5367a4f4c757 Gitweb: http://git.kernel.org/tip/78f71c996fb92d129ec75d572e2f5367a4f4c757 Author: Taeung Song <[email protected]> AuthorDate: Mon, 6 Jun 2016 19:52:52 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 6 Jun 2016 17:43:17 -0300
perf config: Fix abnormal termination at perf_parse_file()
If a config file has wrong key-value pairs, the perf process will be forcibly terminated by die() at perf_parse_file() called by perf_config() so terminal settings can be crushed because of unusual termination.
For example:
If user config file has a wrong value 'red;default' instead of a normal value like 'red, default' for a key 'colors.top',
# cat ~/.perfconfig
[colors]
medium = red;default # wrong valueand if running sub-command 'top',
# perf top
perf process is dead by force and terminal setting is broken with a messge like below.
Fatal: bad config file line 2 in /root/.perfconfig
So fix it.
If perf_config() can return on failure without calling die() at perf_parse_file(), this problem can be solved.
And if a config file has wrong values, show the error message and then use default config values instead of wrong config values.
--- tools/perf/util/config.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
tools/perf/util/config.c +7 -9
@@ -275,7 +275,8 @@ static int perf_parse_file(config_fn_t fn, void *data)break ; } } -die ("bad config file line %d in %s" , config_linenr, config_file_name); +pr_err ("bad config file line %d in %s\n" , config_linenr, config_file_name); +return -1 ; }static int parse_unit_factor (const char *end,unsigned long *val)
@@ -479,16 +480,15 @@ static int perf_config_global(void)int perf_config (config_fn_t fn,void *data) { -int ret =0 , found =0 ; +int ret =-1 ;const char *home =NULL ;/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */ if (config_exclusive_filename)return perf_config_from_file (fn, config_exclusive_filename, data);if (perf_config_system () && !access (perf_etc_perfconfig (),R_OK )) { - ret +=perf_config_from_file (fn,perf_etc_perfconfig (), - data); - found +=1 ; +if (perf_config_from_file (fn,perf_etc_perfconfig (), data) <0 ) + goto out; } home =getenv ("HOME" );
@@ -514,14 +514,12 @@ int perf_config(config_fn_t fn, void *data)if (!st.st_size) goto out_free; - ret +=perf_config_from_file (fn, user_config, data); - found +=1 ; + ret =perf_config_from_file (fn, user_config, data); + out_free:free (user_config); } out: -if (found ==0 ) -return -1 ;return ret; }
PATCH v6 2/9 perf config: If collect_config() is failed, finally free a config set after it is done
In reply to Taeung SongBecause 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().
--- tools/perf/util/config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/config.c +2 -2
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerRe: [BUGFIX][PATCH v6 2/9] perf config: If collect_config() is failed, finally free a config set after it is done
Em Mon, Jun 06, 2016 at 07:52:53PM +0900, Taeung Song escreveu:
Quoted 30 lines
Because of
die()atperf_parse_file()a config set was freed incollect_config(), if failed.
But it is natural to free a config set aftercollect_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 incollect_config().Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>Cc Alexander Shishkin <[email protected]>Signed-off-by Taeung Song <[email protected]>--- tools/perf/util/config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)tools/perf/util/config.c +0
-1@@ -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 ; }
tools/perf/util/config.c struct perf_config_set *perf_config_set__new(void) · line 655
Quoted 9 lines
tools/perf/util/config.c +2
-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?
- Arnaldo
Re: [BUGFIX][PATCH v6 2/9] perf config: If collect_config() is failed, finally free a config set after it is done
Quoted 55 lines · Arnaldo Carvalho de Melo
Em Mon, Jun 06, 2016 at 07:52:53PM +0900, Taeung Song escreveu:
Quoted 30 lines
Because of
die()atperf_parse_file()a config set was freed incollect_config(), if failed.
But it is natural to free a config set aftercollect_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 incollect_config().Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>Cc Alexander Shishkin <[email protected]>Signed-off-by Taeung Song <[email protected]>--- tools/perf/util/config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)tools/perf/util/config.c +0
-1@@ -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 ; }tools/perf/util/config.c struct perf_config_set *perf_config_set__new(void) · line 655
Quoted 9 lines
tools/perf/util/config.c +2
-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);(quoted diff) +4
-1- perf_config(collect_config, set); + if (perf_config(collect_config, set) < 0) { + perf_config_set__delete(set); + set = NULL; + } } return set;
Quoted 2 lines
No?
Granted
Sorry for missing above..
I modified using 'return NULL;' instead of 'set = NULL;' as below
tools/perf/util/config.c +4 -2
@@ -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 :)
Taeung
Re: [BUGFIX][PATCH v6 2/9] perf config: If collect_config() is failed, finally free a config set after it is done
Quoted 94 lines · Taeung Song
Quoted 57 lines · Arnaldo Carvalho de Melo
Em Mon, Jun 06, 2016 at 07:52:53PM +0900, Taeung Song escreveu:
Quoted 32 lines
Because of
die()atperf_parse_file()a config set was freed incollect_config(), if failed.
But it is natural to free a config set aftercollect_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 incollect_config().Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>Cc Alexander Shishkin <[email protected]>Signed-off-by Taeung Song <[email protected]>--- tools/perf/util/config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)tools/perf/util/config.c +0
-1@@ -639,7 +639,6 @@ static int collect_config(const char *var, constchar *value, out_free:free (key ); -perf_config_set__delete (set);return -1 ; }tools/perf/util/config.c struct perf_config_set *perf_config_set__new(void) · line 655
Quoted 9 lines
tools/perf/util/config.c +2
-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);tools/perf/util/config.c +4
-1-perf_config (collect_config, set); +if (perf_config (collect_config, set) <0 ) { +perf_config_set__delete (set); + set =NULL ; + } }return set;Quoted 2 lines
No?
Granted
Sorry for missing above..I modified using 'return NULL;' instead of 'set = NULL;' as below
tools/perf/util/config.c +4
-2@@ -643,7 +643,6 @@ static int collect_config(const char *var, constchar *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;
Quoted 5 lines
Because in near future,
perf_config_set__delete()will usezfree().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
PATCH v6 3/9 perf config: Handle the error when config set is NULL at collect_config()
In reply to Taeung Songcollect_config() collect all config key-value pairs from config files and put each config info in config set. But if config set (i.e. 'set' variable at collect_config()) is NULL, this is wrong so handle it.
--- tools/perf/util/config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/util/config.c +5 -1
@@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,struct perf_config_section *section =NULL ;struct perf_config_item *item =NULL ;struct perf_config_set *set = perf_config_set; -struct list_head *sections = &set->sections; +struct list_head *sections; +if (set ==NULL ) +return -1 ; + + sections = &set->sections; key = ptr =strdup (var);if (!key) {pr_debug ("%s: strdup failed\n" , __func__);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerRe: [BUGFIX][PATCH v6 3/9] perf config: Handle the error when config set is NULL at collect_config()
Em Mon, Jun 06, 2016 at 07:52:54PM +0900, Taeung Song escreveu:
Quoted 4 lines
collect_config()collect all config key-value pairs from config files and put each config info in config set. But if config set (i.e. 'set' variable atcollect_config()) is NULL, this is wrong so handle it.
Looks ok, applied.
- Arnaldo
Quoted 29 lines
Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>Cc Alexander Shishkin <[email protected]>Signed-off-by Taeung Song <[email protected]>--- tools/perf/util/config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)tools/perf/util/config.c +5
-1@@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,struct perf_config_section *section =NULL ;struct perf_config_item *item =NULL ;struct perf_config_set *set = perf_config_set; -struct list_head *sections = &set->sections; +struct list_head *sections; +if (set ==NULL ) +return -1 ; + + sections = &set->sections; key = ptr =strdup (var);if (!key) {pr_debug ("%s: strdup failed\n" , __func__);Signature · 1 line
2.5.0
tip-bot for Taeung Song
merge bot[tip:perf/core] perf config: Handle the error when config set is NULL at collect_config()
In reply to Taeung Songperf config: Handle the error when config set is NULL at collect_config()
Commit-ID: 7db91f251056f90fec4121f028680ab3153a0f3c Gitweb: http://git.kernel.org/tip/7db91f251056f90fec4121f028680ab3153a0f3c Author: Taeung Song <[email protected]> AuthorDate: Mon, 6 Jun 2016 19:52:54 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 6 Jun 2016 17:43:19 -0300
perf config: Handle the error when config set is NULL at collect_config()
collect_config() collect all config key-value pairs from config files and put each config info in config set. But if config set (i.e. 'set' variable at collect_config()) is NULL, this is wrong so handle it.
--- tools/perf/util/config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/util/config.c +5 -1
@@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,struct perf_config_section *section =NULL ;struct perf_config_item *item =NULL ;struct perf_config_set *set = perf_config_set; -struct list_head *sections = &set->sections; +struct list_head *sections; +if (set ==NULL ) +return -1 ; + + sections = &set->sections; key = ptr =strdup (var);if (!key) {pr_debug ("%s: strdup failed\n" , __func__);
PATCH v6 4/9 perf config: Use new perf_config_set__init() to initialize config set
In reply to Taeung SongInstead of perf_config(), This function initialize config set collecting all configs from config files (i.e. user config ~/.perfconfig and system config $(sysconfdir)/perfconfig).
If there are the same config variable both user and system config file, user config has higher priority than system config.
--- tools/perf/util/config.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-)
tools/perf/util/config.c +48 -1
@@ -646,14 +646,61 @@ out_free:return -1 ; } +static int perf_config_set__init (struct perf_config_set *set) +{ +int ret =-1 ; +const char *home =NULL ; + +/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */ +if (config_exclusive_filename) +return perf_config_from_file (collect_config, config_exclusive_filename, set); +if (perf_config_system () && !access (perf_etc_perfconfig (),R_OK )) { +if (perf_config_from_file (collect_config,perf_etc_perfconfig (), set) <0 ) + goto out; + } + + home =getenv ("HOME" ); +if (perf_config_global () && home) { +char *user_config =strdup (mkpath ("%s/.perfconfig" , home)); +struct stat st; + +if (user_config ==NULL ) { +warning ("Not enough memory to process %s/.perfconfig, " +"ignoring it." , home); + goto out; + } + +if (stat (user_config, &st) <0 ) + goto out_free; + +if (st.st_uid && (st.st_uid !=geteuid ())) { +warning ("File %s not owned by current user or root, " +"ignoring it." , user_config); + goto out_free; + } + +if (!st.st_size) + goto out_free; + + ret =perf_config_from_file (collect_config, user_config, set); + +out_free: +free (user_config); + } +out: +return ret; +} +struct perf_config_set *perf_config_set__new (void ) {struct perf_config_set *set =zalloc (sizeof (*set));if (set) {INIT_LIST_HEAD (&set->sections); -if (perf_config (collect_config, set) <0 ) +if (perf_config_set__init (set) <0 ) {perf_config_set__delete (set); +return NULL ; + } }return set;
Signature · 1 line
2.5.0
PATCH v6 5/9 perf config: Add global variable 'config_set'
In reply to Taeung SongThe config set is prepared by collecting
all configs from config files (i.e. user config ~/.perfconfig and system config $(sysconfdir)/perfconfig) so the config set contains all config key-value pairs.
We need to use it as global variable to share it.
And in near future, the variable will be handled in perf_config() and other functions at util/config.c
--- tools/perf/builtin-config.c | 9 ++++----- tools/perf/util/config.c | 1 + tools/perf/util/config.h | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-)
tools/perf/builtin-config.c +4 -5
@@ -62,7 +62,6 @@ static int show_config(struct perf_config_set *set)int cmd_config (int argc,const char **argv,const char *prefix__maybe_unused ) {int ret =0 ; -struct perf_config_set *set;char *user_config =mkpath ("%s/.perfconfig" ,getenv ("HOME" )); argc = parse_options(argc, argv, config_options, config_usage,
@@ -80,8 +79,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)else if (use_user_config) config_exclusive_filename = user_config; - set =perf_config_set__new (); -if (!set) { + config_set =perf_config_set__new (); +if (!config_set) { ret =-1 ; goto out_err; }
@@ -92,7 +91,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)pr_err ("Error: takes no arguments\n" );parse_options_usage (config_usage, config_options,"l" ,1 ); }else { - ret =show_config (set); + ret =show_config (config_set);if (ret <0 ) {const char * config_filename = config_exclusive_filename;if (!config_exclusive_filename)
@@ -106,7 +105,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)usage_with_options (config_usage, config_options); } -perf_config_set__delete (set); +perf_config_set__delete (config_set); out_err:return ret; }
tools/perf/util/config.c +1 -0
@@ -28,6 +28,7 @@ static int config_linenr;static int config_file_eof;const char *config_exclusive_filename; +struct perf_config_set *config_set;static int get_next_char (void ) {
tools/perf/util/config.h +2 -0
@@ -20,6 +20,8 @@ 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 (struct perf_config_set *set);
Signature · 1 line
2.5.0
PATCH v6 6/9 perf config: Use zfree() instead of free() at perf_config_set__delete()
In reply to Taeung Songperf_config_set__delete() delete allocated the config set but the global variable 'config_set' is used all around.
So purge and zfree by an address of the global variable , i.e. 'struct perf_config_set **' type instead of using local variable 'set' of which type is 'struct perf_config_set *'.
--- tools/perf/builtin-config.c | 2 +- tools/perf/util/config.c | 11 +++++++---- tools/perf/util/config.h | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-)
tools/perf/builtin-config.c +1 -1
@@ -105,7 +105,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)usage_with_options (config_usage, config_options); } -perf_config_set__delete (config_set); +perf_config_set__delete (&config_set); out_err:return ret; }
tools/perf/util/config.c +7 -4
@@ -699,7 +699,7 @@ struct perf_config_set *perf_config_set__new(void)if (set) {INIT_LIST_HEAD (&set->sections);if (perf_config_set__init (set) <0 ) { -perf_config_set__delete (set); +perf_config_set__delete (&set);return NULL ; } }
@@ -741,10 +741,13 @@ static void perf_config_set__purge(struct perf_config_set *set) } } -void perf_config_set__delete (struct perf_config_set *set) +void perf_config_set__delete (struct perf_config_set **set) { -perf_config_set__purge (set); -free (set); +if (*set ==NULL ) +return ; + +perf_config_set__purge (*set); +zfree (set); }/*
tools/perf/util/config.h +1 -1
@@ -23,6 +23,6 @@ struct perf_config_set {extern struct perf_config_set *config_set;struct perf_config_set *perf_config_set__new (void ); -void perf_config_set__delete (struct perf_config_set *set); +void perf_config_set__delete (struct perf_config_set **set);#endif /* __PERF_CONFIG_H */
Signature · 1 line
2.5.0
PATCH v6 7/9 perf config: Reimplement perf_config() using perf_config_set__iter()
In reply to Taeung SongEverytime perf_config() is called, perf_config() always read config files. (i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')
But we need to use the config set that already contains all config key-value pairs to avoid this repetitive work reading the config files in perf_config(). (the config set mean a global variable 'config_set')
In other words, if new perf_config() is called,
only first time 'config_set' is initialized collecting all configs from the config files and it work with perf_config_set__iter(). And free the config set after sub-command work at run_builtin().
If we do, 'config_set' can be reused wherever using perf_config() and a feature of old perf_config() is the same as new perf_config() work without the repetitive work that read the config files.
--- tools/perf/perf.c | 1 + tools/perf/util/cache.h | 1 + tools/perf/util/config.c | 86 +++++++++++++++++++++++------------------------- 3 files changed, 43 insertions(+), 45 deletions(-)
tools/perf/perf.c +1 -0
@@ -391,6 +391,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)perf_env__set_cmdline (&perf_env, argc, argv); status = p->fn (argc, argv, prefix); +perf_config_set__delete (&config_set);exit_browser (status);perf_env__exit (&perf_env);bpf__clear ();
tools/perf/util/cache.h +1 -0
@@ -7,6 +7,7 @@#include <subcmd/pager.h> #include "../perf.h" #include "../ui/ui.h" +#include "config.h" #include <linux/string.h>
tools/perf/util/config.c +41 -45
98 lines not on this page · raw message
Signature · 1 line
2.5.0
PATCH v6 8/9 perf config: Reset the config set at only 'config' sub-command
In reply to Taeung SongWhen first calling perf_config(), the config set is initialized collecting both user and system config files (i.e. user config ~/.perfconfig and system config $(sysconfdir)/perfconfig) so config set contains not only user config but also system config key-value pairs. (User config has higher priority than system config.)
But 'config' sub-command individually use the config set so free the existing config set (i.e. a global variable config_set) before reinstantiating it.
And 'config' sub-command have '--user' or '--system' options. To reinitialize with the options, the config set should be reset at the very beginning at cmd_config()
--- tools/perf/builtin-config.c | 5 +++++ 1 file changed, 5 insertions(+)
tools/perf/builtin-config.c +5 -0
@@ -79,6 +79,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)else if (use_user_config) config_exclusive_filename = user_config; +/* +* Reset the config set at only 'config' sub-command +* because of reinitializing with options config file location. +*/ +perf_config_set__delete (&config_set); config_set =perf_config_set__new ();if (!config_set) { ret =-1 ;
Signature · 1 line
2.5.0
PATCH v6 9/9 perf config: Reimplement show_config() using perf_config()
In reply to Taeung SongOld show_config() directly use config set so
there are many duplicated code with perf_config_set__iter().
So reimplement show_config() using perf_config() that use perf_config_set__iter() with config set that already contains all configs.
--- tools/perf/builtin-config.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-)
tools/perf/builtin-config.c +7 -22
@@ -33,28 +33,13 @@ static struct option config_options[] = { OPT_END() }; -static int show_config (struct perf_config_set *set) +static int show_config (const char *key,const char *value, +void *cb__maybe_unused ) { -struct perf_config_section *section; -struct perf_config_item *item; -struct list_head *sections; - -if (set ==NULL ) -return -1 ; - - sections = &set->sections; -if (list_empty (sections)) -return -1 ; - -list_for_each_entry (section, sections, node) { -list_for_each_entry (item, §ion->items, node) { -char *value = item->value; - -if (value) -printf ("%s.%s=%s\n" , section->name, - item->name, value); - } - } +if (value) +printf ("%s=%s\n" , key, value); +else +printf ("%s\n" , key);return 0 ; }
@@ -96,7 +81,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)pr_err ("Error: takes no arguments\n" );parse_options_usage (config_usage, config_options,"l" ,1 ); }else { - ret =show_config (config_set); + ret =perf_config (show_config,NULL );if (ret <0 ) {const char * config_filename = config_exclusive_filename;if (!config_exclusive_filename)
Signature · 1 line
2.5.0