Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config

[Arnaldo Carvalho de Melo]

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@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
> ---
>  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