Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 15 Dec 2020 12:41:18 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Jiri Olsa <jolsa@kernel.org>
Cc:     lkml <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Michael Petlan <mpetlan@redhat.com>,
        Ian Rogers <irogers@google.com>,
        Stephane Eranian <eranian@google.com>,
        Alexei Budankov <abudankov@huawei.com>
Subject: Re: [PATCH 3/8] perf tools: Add config set interface
Message-ID: <20201215154118.GD258566@kernel.org>
References: <20201212104358.412065-1-jolsa@kernel.org>
 <20201212104358.412065-4-jolsa@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201212104358.412065-4-jolsa@kernel.org>
Precedence: bulk

Em Sat, Dec 12, 2020 at 11:43:53AM +0100, Jiri Olsa escreveu:
> Add interface to load config set from custom file
> by using perf_config_set__new_file function.
> 
> It will be used in perf daemon command to process
> custom config file.

The naming may be misleading, as this is not _setting_ the perf config
to be that one, but to apply a set of changes to whatever is the current
configuration, right?

Perhaps 'perf_config_set__load_file()'

There is value in _resetting_ the config to some configuration, i.e.
moving everything to the strict defaults and then loading a file that
sets the configuration to a state similar to what perf does when it
first starts.

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/config.c | 28 +++++++++++++++++++++++-----
>  tools/perf/util/config.h |  3 +++
>  2 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 6969f82843ee..dc3f03f8bbf5 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -738,6 +738,18 @@ struct perf_config_set *perf_config_set__new(void)
>  	return set;
>  }
>  
> +struct perf_config_set *perf_config_set__new_file(const char *file)
> +{
> +	struct perf_config_set *set = zalloc(sizeof(*set));
> +
> +	if (set) {
> +		INIT_LIST_HEAD(&set->sections);
> +		perf_config_from_file(collect_config, file, set);
> +	}
> +
> +	return set;
> +}
> +
>  static int perf_config__init(void)
>  {
>  	if (config_set == NULL)
> @@ -746,17 +758,15 @@ static int perf_config__init(void)
>  	return config_set == NULL;
>  }
>  
> -int perf_config(config_fn_t fn, void *data)
> +int perf_config_set(struct perf_config_set *set,
> +		    config_fn_t fn, void *data)
>  {
>  	int ret = 0;
>  	char key[BUFSIZ];
>  	struct perf_config_section *section;
>  	struct perf_config_item *item;
>  
> -	if (config_set == NULL && perf_config__init())
> -		return -1;
> -
> -	perf_config_set__for_each_entry(config_set, section, item) {
> +	perf_config_set__for_each_entry(set, section, item) {
>  		char *value = item->value;
>  
>  		if (value) {
> @@ -778,6 +788,14 @@ int perf_config(config_fn_t fn, void *data)
>  	return ret;
>  }
>  
> +int perf_config(config_fn_t fn, void *data)
> +{
> +	if (config_set == NULL && perf_config__init())
> +		return -1;
> +
> +	return perf_config_set(config_set, fn, data);
> +}
> +
>  void perf_config__exit(void)
>  {
>  	perf_config_set__delete(config_set);
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 8c881e3a3ec3..f58b457e7e5f 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -30,6 +30,8 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
>  int perf_config_from_file(config_fn_t fn, const char *filename, void *data);
>  int perf_default_config(const char *, const char *, void *);
>  int perf_config(config_fn_t fn, void *);
> +int perf_config_set(struct perf_config_set *set,
> +		    config_fn_t fn, void *data);
>  int perf_config_int(int *dest, const char *, const char *);
>  int perf_config_u8(u8 *dest, const char *name, const char *value);
>  int perf_config_u64(u64 *dest, const char *, const char *);
> @@ -38,6 +40,7 @@ int config_error_nonbool(const char *);
>  const char *perf_etc_perfconfig(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
> +struct perf_config_set *perf_config_set__new_file(const char *file);
>  void perf_config_set__delete(struct perf_config_set *set);
>  int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
>  			     const char *var, const char *value);
> -- 
> 2.26.2
> 

-- 

- Arnaldo