lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Jun 2016 20:05:39 +0900 From: Taeung Song <treeze.taeung@...il.com> To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com> 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>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Masami Hiramatsu <mhiramat@...nel.org>, Wang Nan <wangnan0@...wei.com>, Jiri Olsa <jolsa@...hat.com>, Ingo Molnar <mingo@...hat.com> Subject: Re: [PATCH v8 3/5] perf config: Reimplement perf_config() using perf_config_set__iter() On 06/09/2016 10:41 PM, Arnaldo Carvalho de Melo wrote: > Em Wed, Jun 08, 2016 at 09:36:51PM +0900, Taeung Song escreveu: >> Many sub-commands use perf_config() so >> 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 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 a 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. >> >> Cc: Namhyung Kim <namhyung@...nel.org> >> Cc: Jiri Olsa <jolsa@...hat.com> >> Cc: Wang Nan <wangnan0@...wei.com> >> Cc: Peter Zijlstra <peterz@...radead.org> >> Cc: Ingo Molnar <mingo@...hat.com> >> Cc: Masami Hiramatsu <mhiramat@...nel.org> >> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com> >> Signed-off-by: Taeung Song <treeze.taeung@...il.com> >> --- >> tools/perf/builtin-config.c | 4 +++ >> tools/perf/perf.c | 1 + >> tools/perf/util/config.c | 87 ++++++++++++++++++++++----------------------- >> tools/perf/util/config.h | 1 + >> 4 files changed, 48 insertions(+), 45 deletions(-) >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index fe1b77f..cfd1036 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -80,6 +80,10 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> else if (use_user_config) >> config_exclusive_filename = user_config; >> >> + /* >> + * At only 'config' sub-command, individually use the config set >> + * because of reinitializing with options config file location. >> + */ >> set = perf_config_set__new(); >> if (!set) { >> ret = -1; >> diff --git a/tools/perf/perf.c b/tools/perf/perf.c >> index 15982ce..fe2ab7c 100644 >> --- a/tools/perf/perf.c >> +++ b/tools/perf/perf.c >> @@ -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(); >> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >> index 31e09a4..72db134 100644 >> --- a/tools/perf/util/config.c >> +++ b/tools/perf/util/config.c >> @@ -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) >> { >> @@ -478,51 +479,6 @@ static int perf_config_global(void) >> return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0); >> } >> >> -int perf_config(config_fn_t fn, void *data) >> -{ >> - 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)) { >> - if (perf_config_from_file(fn, perf_etc_perfconfig(), data) < 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(fn, user_config, data); >> - >> -out_free: >> - free(user_config); >> - } >> -out: >> - return ret; >> -} >> - >> static struct perf_config_section *find_section(struct list_head *sections, >> const char *section_name) >> { >> @@ -706,6 +662,47 @@ struct perf_config_set *perf_config_set__new(void) >> return set; >> } >> >> +static int perf_config_set__iter(struct perf_config_set *set, config_fn_t fn, void *data) >> +{ >> + struct perf_config_section *section; >> + struct perf_config_item *item; >> + struct list_head *sections; >> + char key[BUFSIZ]; >> + >> + 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) { >> + scnprintf(key, sizeof(key), "%s.%s", >> + section->name, item->name); >> + if (fn(key, value, data) < 0) { >> + pr_err("Error: wrong config key-value pair %s=%s\n", >> + key, value); >> + return -1; >> + } >> + } >> + } >> + } >> + >> + return 0; >> +} >> + >> +int perf_config(config_fn_t fn, void *data) >> +{ >> + if (config_set == NULL) >> + config_set = perf_config_set__new(); >> + >> + return perf_config_set__iter(config_set, fn, data); >> +} >> + > > Try not using those iter + callback things, use something like this > instead: > > perf_config_set__for_each(pos, config_set) > fn(pos, data) > > I.e. use the list.h model. At some point this perf_config() function > should probably die, I guess. > Granted!! Will change perf_config_set__iter() to perf_config_set__for_each() as you said!! Thanks, Taeung
Powered by blists - more mailing lists