[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160509171717.GA13209@kernel.org>
Date: Mon, 9 May 2016 14:17:17 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Taeung Song <treeze.taeung@...il.com>
Cc: linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Subject: Re: [PATCH 2/4] perf tools: Separate out code setting ground colors
from ui_browser__color_config
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@...nel.org>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Signed-off-by: Taeung Song <treeze.taeung@...il.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
Powered by blists - more mailing lists