lkml.org 
[lkml]   [2016]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
From
Date
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 <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()?

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

\
 
 \ /
  Last update: 2016-05-10 14:01    [W:0.057 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site