From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751275AbcEIRR3 (ORCPT <rfc822;w@1wt.eu>);
	Mon, 9 May 2016 13:17:29 -0400
Received: from mail.kernel.org ([198.145.29.136]:47796 "EHLO mail.kernel.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750914AbcEIRR2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Mon, 9 May 2016 13:17:28 -0400
Date: Mon, 9 May 2016 14:17:17 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Taeung Song <treeze.taeung@gmail.com>
Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>
Subject: Re: [PATCH 2/4] perf tools: Separate out code setting ground colors
 from ui_browser__color_config
Message-ID: <20160509171717.GA13209@kernel.org>
References: <1462794109-14652-1-git-send-email-treeze.taeung@gmail.com>
 <1462794109-14652-3-git-send-email-treeze.taeung@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1462794109-14652-3-git-send-email-treeze.taeung@gmail.com>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.6.0 (2016-04-01)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.

> But it can be reused by other functions so make ui_browser__config_gcolors()
> bringing it from ui_browser__color_config().
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  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()?

> +
> +	ui_browser_color->fg = fg;
> +	ui_browser_color->bg = bg;
> +	return 0;
> +}
>  
>  static int ui_browser__color_config(const char *var, const char *value,
>  				    void *data __maybe_unused)
>  {
> -	char *fg = NULL, *bg;
> -	int i;
> +	int i, ret;
>  
>  	/* same dir for all commands */
>  	if (prefixcmp(var, "colors.") != 0)
> @@ -570,23 +591,11 @@ 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;
> -
> -		bg = strchr(fg, ',');
> -		if (bg == NULL)
> -			break;
> -
> -		*bg = '\0';
> -		while (isspace(*++bg));

strtok + ltrim

> -		ui_browser__colorsets[i].bg = bg;
> -		ui_browser__colorsets[i].fg = fg;
> -		return 0;
> +		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
> +		break;
>  	}
>  
> -	free(fg);
> -	return -1;
> +	return ret;
>  }
>  
>  void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
> -- 
> 2.5.0