Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934062AbcK2QF6 (ORCPT ); Tue, 29 Nov 2016 11:05:58 -0500 Received: from mail.kernel.org ([198.145.29.136]:60944 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751227AbcK2QFs (ORCPT ); Tue, 29 Nov 2016 11:05:48 -0500 Date: Tue, 29 Nov 2016 13:05:38 -0300 From: Arnaldo Carvalho de Melo To: Taeung Song Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Wang Nan , Jiri Olsa , Ingo Molnar , Masami Hiramatsu , David Ahern Subject: Re: [PATCH v9 1/7] perf config: Introduce default_config_section and default_config_item for default config key-value pairs Message-ID: <20161129160538.GB23423@kernel.org> References: <1480323078-15623-1-git-send-email-treeze.taeung@gmail.com> <1480323078-15623-2-git-send-email-treeze.taeung@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480323078-15623-2-git-send-email-treeze.taeung@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2694 Lines: 88 Em Mon, Nov 28, 2016 at 05:51:12PM +0900, Taeung Song escreveu: > When initializing default perf config values, > we currently use values of actual type(int, bool, char *, etc.). > > For example, > If there isn't a user config value for 'annotate.use_offset' > config variable at ~/.perfconfig, > default value for it is 'true' bool type value in perf like below. > > At ui/browsers/annoate.c > > static struct annotate_browser_opt { > bool hide_src_code, > use_offset, > jump_arrows, > show_linenr, > show_nr_jumps, > show_total_period; > } annotate_browser__opts = { > .use_offset = true, > .jump_arrows = true, > }; And isn't this a good thing? I.e. its compact, we have just to set values to things that are !0 (or !false), and it is close to the code where it is used. > But I suggest using 'struct default_config_section' and > 'struct default_config_item' that can contain default config key-value pairs > in order to initialize default config values with them, in near future. > If we do, we could manage default perf config values at one spot (i.e. util/config.c) > with default config arrays and it could be easy and simple to modify > existing default config values or add default values for new config item. > > Cc: Namhyung Kim > Cc: Jiri Olsa > Cc: Wang Nan > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Masami Hiramatsu > Cc: David Ahern > Signed-off-by: Taeung Song > --- > tools/perf/util/config.h | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h > index 1a59a6b..434d71c 100644 > --- a/tools/perf/util/config.h > +++ b/tools/perf/util/config.h > @@ -67,4 +67,33 @@ void perf_config__refresh(void); > perf_config_sections__for_each_entry(&set->sections, section) \ > perf_config_items__for_each_entry(§ion->items, item) > > +enum perf_config_type { > + CONFIG_TYPE_BOOL, > + CONFIG_TYPE_INT, > + CONFIG_TYPE_LONG, > + CONFIG_TYPE_U64, > + CONFIG_TYPE_FLOAT, > + CONFIG_TYPE_DOUBLE, > + CONFIG_TYPE_STRING > +}; > + > +struct default_config_item { > + const char *name; > + union { > + bool b; > + int i; > + u32 l; > + u64 ll; > + float f; > + double d; > + const char *s; > + } value; > + enum perf_config_type type; > +}; > + > +struct default_config_section { > + const char *name; > + const struct default_config_item *items; > +}; > + > #endif /* __PERF_CONFIG_H */ > -- > 2.7.4