Messages in this thread | | | Date | Tue, 31 May 2016 10:43:24 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v4 1/6] perf config: Use new perf_config_set__init() to initialize config set |
| |
Em Tue, May 31, 2016 at 10:13:43AM +0900, Taeung Song escreveu: > Instead 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. > > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Signed-off-by: Taeung Song <treeze.taeung@gmail.com> > --- > tools/perf/util/config.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 49 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > index dad7d82..5d01899 100644 > --- a/tools/perf/util/config.c > +++ b/tools/perf/util/config.c > @@ -645,13 +645,61 @@ out_free: > return -1; > } > > +static int perf_config_set__init(struct perf_config_set *set) > +{ > + int ret = 0, found = 0; > + 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)) { > + ret += perf_config_from_file(collect_config, perf_etc_perfconfig(), set); > + found += 1; > + } > + > + 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); > + found += 1; > +out_free: > + free(user_config); > + } > +out: > + if (found == 0) > + return -1; > + 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); > - perf_config(collect_config, set); > + if (perf_config_set__init(set) < 0) > + return NULL;
So, the usual pattern is: alloc, init, fail? free, return NULL.
I thought you could've been deviating from that pattern and went to look at perf_config_set__init() to see if that was doing the freeing in case of failure, which it shouldn't, it isn't, so I guess this is a leak on failure, no?
- Arnaldo
| |