Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752015AbcDTNWn (ORCPT ); Wed, 20 Apr 2016 09:22:43 -0400 Received: from mail.kernel.org ([198.145.29.136]:33452 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750767AbcDTNWm (ORCPT ); Wed, 20 Apr 2016 09:22:42 -0400 Date: Wed, 20 Apr 2016 10:22:36 -0300 From: Arnaldo Carvalho de Melo To: Namhyung Kim Cc: Taeung Song , Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra , Masami Hiramatsu Subject: Re: [PATCH v8 3/4] perf config: Prepare all default configs Message-ID: <20160420132236.GM3677@kernel.org> References: <1460620401-23430-1-git-send-email-treeze.taeung@gmail.com> <1460620401-23430-4-git-send-email-treeze.taeung@gmail.com> <20160414121916.GJ9056@kernel.org> <570FC86D.5060909@gmail.com> <5714F556.1000308@gmail.com> <20160420124438.GA29186@danjae.aot.lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160420124438.GA29186@danjae.aot.lge.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1923 Lines: 64 Em Wed, Apr 20, 2016 at 09:44:38PM +0900, Namhyung Kim escreveu: > On Mon, Apr 18, 2016 at 11:55:18PM +0900, Taeung Song wrote: > > On 04/15/2016 01:42 AM, Taeung Song wrote: > > > On 04/14/2016 09:19 PM, Arnaldo Carvalho de Melo wrote: > > > > Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu: > > > > > +++ b/tools/perf/util/config.c > > > > > @@ -15,6 +15,7 @@ > > > > > +#define MAX_CONFIGS 64 > > > > Do we have to add another arbitrary maximum? Where is it used? > > > IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, > > > I'd modify the code. > > > > > > MAX_CONFIGS is used in order to declare two-dimensional arrays > > > 'default_config_items' > > As above, I used MAX_CONFIGS because of two-dimensinal arrays > > 'default_config_items'. > > What do you think about it ? > I also agree that we'd better to avoid the arbitrary maximum. > > We don't need to add this arbitrary maximum ? > > or would you mind, if I look for other way about > > 'default_config_item' ? > > What about this? Yeah, I guess this should work, no? At least this is how it is done elsewhere, see: tools/perf/builtin-bench.c, 'struct collection' has a benchmarks array, that in turn is organized as Namhyung suggests. Then you either use ARRAY_SIZE() somewhere to get the number of entries or use a sentinel, i.e. use NULL for the last entry. - Arnaldo > struct perf_config_item color_config_items[] = { > CONF_STR_VAR("top", "red, default"), > CONF_STR_VAR("medium", "green, default"), > ... > }; > > struct perf_config_item tui_config_items[] = { > CONF_BOOL_VAR("report", true), > CONF_BOOL_VAR("annotate", true), > ... > }; > > struct perf_config_item *default_config_items[] = { > &color_config_items, > &tui_config_items, > ... > }; > > This way we can access the config array by using constant index > without the hard-coded maximum size IMHO. > > Thanks, > Namhyung