Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751787AbcDTP3W (ORCPT ); Wed, 20 Apr 2016 11:29:22 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33220 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751238AbcDTP3U (ORCPT ); Wed, 20 Apr 2016 11:29:20 -0400 Subject: Re: [PATCH v8 3/4] perf config: Prepare all default configs To: Arnaldo Carvalho de Melo , Namhyung Kim 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> <20160420132236.GM3677@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra , Masami Hiramatsu From: Taeung Song Message-ID: <5717A04C.1080403@gmail.com> Date: Thu, 21 Apr 2016 00:29:16 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160420132236.GM3677@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2203 Lines: 77 Hi, Arnaldo :-) On 04/20/2016 10:22 PM, Arnaldo Carvalho de Melo wrote: > 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. I got it! > 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. > I think that it would better to use ARRAY_SIZE() than to use NULL. It makes no odds but it seem to be clean, IMHO. :) After changing this patchset, I'll send v9 ! Thanks, Taeung > >> 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