From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751552AbcEJLdK (ORCPT ); Tue, 10 May 2016 07:33:10 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:33284 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751370AbcEJLdJ (ORCPT ); Tue, 10 May 2016 07:33:09 -0400 Subject: Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config To: Arnaldo Carvalho de Melo References: <1462794109-14652-1-git-send-email-treeze.taeung@gmail.com> <1462794109-14652-3-git-send-email-treeze.taeung@gmail.com> <20160509171717.GA13209@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Alexander Shishkin From: Taeung Song Message-ID: <5731C6F0.9000706@gmail.com> Date: Tue, 10 May 2016 20:33:04 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20160509171717.GA13209@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 05/10/2016 02:17 AM, Arnaldo Carvalho de Melo wrote: > Em Mon, May 09, 2016 at 08:41:47PM +0900, Taeung Song escreveu: >> ui_browser__color_config() set foreground and background >> colors values in ui_browser__colorsets. > > "ground colors" sounds strange, I guess referreing to them as "*colors" > or "{back,fore}ground colors" is more appropriate, replace "gcolors" > with "colors" too, please. I got it. >> But it can be reused by other functions so make ui_browser__config_gcolors() >> bringing it from ui_browser__color_config(). >> >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Signed-off-by: Taeung Song >> --- >> tools/perf/ui/browser.c | 43 ++++++++++++++++++++++++++----------------- >> 1 file changed, 26 insertions(+), 17 deletions(-) >> >> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c >> index af68a9d..a477867 100644 >> --- a/tools/perf/ui/browser.c >> +++ b/tools/perf/ui/browser.c >> @@ -553,12 +553,33 @@ static struct ui_browser_colorset { >> } >> }; >> >> +static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color, >> + const char *value) >> +{ >> + char *fg = NULL, *bg; >> + >> + fg = strdup(value); >> + if (fg == NULL) >> + return -1; >> + >> + bg = strchr(fg, ','); >> + if (bg == NULL) { >> + free(fg); >> + return -1; >> + } >> + >> + *bg = '\0'; > > Isn't the above strtok()? > >> + while (isspace(*++bg)); > > Isn't this ltrim()? I modified it like below and retested it ! diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c index a477867..c905445 100644 --- a/tools/perf/ui/browser.c +++ b/tools/perf/ui/browser.c @@ -553,8 +553,8 @@ static struct ui_browser_colorset { } }; -static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color, - const char *value) +static int ui_browser__config_colors(struct ui_browser_colorset *ui_browser_color, + const char *value) { char *fg = NULL, *bg; @@ -562,14 +562,12 @@ static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_col if (fg == NULL) return -1; - bg = strchr(fg, ','); + bg = strtok(fg, ","); if (bg == NULL) { free(fg); return -1; } - - *bg = '\0'; - while (isspace(*++bg)); + ltrim(bg); ui_browser_color->fg = fg; ui_browser_color->bg = bg; @@ -591,7 +589,7 @@ static int ui_browser__color_config(const char *var, const char *value, if (strcmp(ui_browser__colorsets[i].name, name) != 0) continue; - ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value); + ret = ui_browser__config_colors(&ui_browser__colorsets[i], value); break; } I'll send this modified patch with v2 Thanks, Taeung