From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751324AbcHIElz (ORCPT ); Tue, 9 Aug 2016 00:41:55 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:35820 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750745AbcHIElx (ORCPT ); Tue, 9 Aug 2016 00:41:53 -0400 Subject: Re: [PATCH v6 RESEND 4/7] perf config: Use combined {fore,back}ground colors value instead of each two color To: Arnaldo Carvalho de Melo References: <1470129649-11957-1-git-send-email-treeze.taeung@gmail.com> <1470129649-11957-5-git-send-email-treeze.taeung@gmail.com> <20160808185824.GE7122@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Masami Hiramatsu , Wang Nan From: Taeung Song Message-ID: Date: Tue, 9 Aug 2016 13:41:47 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160808185824.GE7122@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 Hi, Arnaldo :) On 08/09/2016 03:58 AM, Arnaldo Carvalho de Melo wrote: > Em Tue, Aug 02, 2016 at 06:20:46PM +0900, Taeung Song escreveu: >> To easily set default config values into actual variables for 'colors' config, >> it would be better that actual variables for each 'colors' config >> also have only one value like 'default_config_item' type. >> >> If we use combined {fore,back}ground colors values in ui_browser_colorset, >> it smoothly work to initialize default config values for 'colors' config >> by 'colors_config_items' array that contains default values for it at util/config.c. >> because both actual variable and config item of 'colors_config_items' >> are equal in the number of values (as just one). >> >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Cc: Masami Hiramatsu >> Cc: Wang Nan >> Signed-off-by: Taeung Song >> --- >> tools/perf/ui/browser.c | 53 +++++++++++++++++++++++-------------------------- >> 1 file changed, 25 insertions(+), 28 deletions(-) >> >> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c >> index 3eb3edb..31e2028 100644 >> --- a/tools/perf/ui/browser.c >> +++ b/tools/perf/ui/browser.c >> @@ -503,61 +503,53 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *browser) >> } >> >> static struct ui_browser_colorset { >> - const char *name, *fg, *bg; >> + const char *name, *colors; >> int colorset; >> } ui_browser__colorsets[] = { >> { >> .colorset = HE_COLORSET_TOP, >> .name = "top", >> - .fg = "red", >> - .bg = "default", >> + .colors = "red, default", >> }, >> { >> .colorset = HE_COLORSET_MEDIUM, >> .name = "medium", >> - .fg = "green", >> - .bg = "default", >> + .colors = "green, default", >> }, >> { >> .colorset = HE_COLORSET_NORMAL, >> .name = "normal", >> - .fg = "default", >> - .bg = "default", >> + .colors = "default, default", >> }, >> { >> .colorset = HE_COLORSET_SELECTED, >> .name = "selected", >> - .fg = "black", >> - .bg = "yellow", >> + .colors = "black, yellow", >> }, >> { >> .colorset = HE_COLORSET_JUMP_ARROWS, >> .name = "jump_arrows", >> - .fg = "blue", >> - .bg = "default", >> + .colors = "blue, default", >> }, >> { >> .colorset = HE_COLORSET_ADDR, >> .name = "addr", >> - .fg = "magenta", >> - .bg = "default", >> + .colors = "magenta, default", >> }, >> { >> .colorset = HE_COLORSET_ROOT, >> .name = "root", >> - .fg = "white", >> - .bg = "blue", >> + .colors = "white, blue", >> }, >> { >> .name = NULL, >> } >> }; >> >> - >> static int ui_browser__color_config(const char *var, const char *value, >> void *data __maybe_unused) >> { >> - char *fg = NULL, *bg; >> + char *colors; >> int i; >> >> /* same dir for all commands */ >> @@ -570,22 +562,18 @@ static int ui_browser__color_config(const char *var, const char *value, >> if (strcmp(ui_browser__colorsets[i].name, name) != 0) >> continue; >> >> - fg = strdup(value); >> - if (fg == NULL) >> - break; >> + if (strstr(value, ",") == NULL) >> + return -1; >> >> - bg = strchr(fg, ','); >> - if (bg == NULL) >> + colors = strdup(value); >> + if (colors == NULL) >> break; >> + ui_browser__colorsets[i].colors = colors; >> >> - *bg = '\0'; >> - while (isspace(*++bg)); >> - ui_browser__colorsets[i].bg = bg; >> - ui_browser__colorsets[i].fg = fg; >> return 0; >> } >> >> - free(fg); >> + free(colors); >> return -1; >> } >> >> @@ -743,8 +731,17 @@ void ui_browser__init(void) >> perf_config(ui_browser__color_config, NULL); >> >> while (ui_browser__colorsets[i].name) { >> + char *colors, *fg, *bg; >> struct ui_browser_colorset *c = &ui_browser__colorsets[i++]; >> - sltt_set_color(c->colorset, c->name, c->fg, c->bg); >> + >> + colors = strdup(c->colors); >> + if (fg == NULL) > > Huh? At this point fb is not even initialized Sorry for my mistake. I'll change 'fg' to 'colors' to handle a exception of strdup(). Thanks, Taeung >> + break; >> + fg = strtok(colors, ","); >> + bg = strtok(NULL, ","); >> + bg = ltrim(bg); >> + sltt_set_color(c->colorset, c->name, fg, bg); >> + free(colors); >> } >> >> annotate_browser__init(); >> -- >> 2.5.0