LinuxLists.cc - [PATCH 0/5] Refactoring with ltrim() and rtrim()

[permalink] [raw]

Subject: [PATCH 5/5] perf tools: Refactor the code to strip command name with {l,r}trim()

After reading command name from /proc/<pid>/status,
use ltrim() and rtrim() to strip command name, not using
just while loop, isspace() and etc.

Cc: David Ahern <[email protected]>
Cc: Don Zickus <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/util/event.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 76b9c6b..8255a26 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -106,7 +106,7 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
int fd;
size_t size = 0;
ssize_t n;
- char *nl, *name, *tgids, *ppids;
+ char *name, *tgids, *ppids;

*tgid = -1;
*ppid = -1;
@@ -134,14 +134,7 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,

if (name) {
name += 5; /* strlen("Name:") */
-
- while (*name && isspace(*name))
- ++name;
-
- nl = strchr(name, '\n');
- if (nl)
- *nl = '\0';
-
+ name = rtrim(ltrim(name));
size = strlen(name);
if (size >= len)
size = len - 1;
--
2.7.4

2017-04-07 14:24:57

[permalink] [raw]

Subject: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

When parsing disassemble lines,
use ltrim() and rtrim() to strip them,
not using just while loop and isspace().

Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/util/annotate.c | 49 ++++++++++------------------------------------
1 file changed, 10 insertions(+), 39 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a37032b..1b4f17b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -379,9 +379,7 @@ static int mov__parse(struct arch *arch, struct ins_operands *ops, struct map *m
if (comment == NULL)
return 0;

- while (comment[0] != '\0' && isspace(comment[0]))
- ++comment;
-
+ comment = ltrim(comment);
comment__symbol(ops->source.raw, comment, &ops->source.addr, &ops->source.name);
comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);

@@ -426,9 +424,7 @@ static int dec__parse(struct arch *arch __maybe_unused, struct ins_operands *ops
if (comment == NULL)
return 0;

- while (comment[0] != '\0' && isspace(comment[0]))
- ++comment;
-
+ comment = ltrim(comment);
comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);

return 0;
@@ -777,10 +773,7 @@ static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, str

static int disasm_line__parse(char *line, const char **namep, char **rawp)
{
- char *name = line, tmp;
-
- while (isspace(name[0]))
- ++name;
+ char tmp, *name = ltrim(line);

if (name[0] == '\0')
return -1;
@@ -798,12 +791,7 @@ static int disasm_line__parse(char *line, const char **namep, char **rawp)
goto out_free_name;

(*rawp)[0] = tmp;
-
- if ((*rawp)[0] != '\0') {
- (*rawp)++;
- while (isspace((*rawp)[0]))
- ++(*rawp);
- }
+ *rawp = ltrim(*rawp);

return 0;

@@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
{
struct annotation *notes = symbol__annotation(sym);
struct disasm_line *dl;
- char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
+ char *line = NULL, *parsed_line, *tmp, *tmp2;
size_t line_len;
- s64 line_ip, offset = -1;
+ s64 line_ip = -1, offset = -1;
regmatch_t match[2];

if (getline(&line, &line_len, file) < 0)
@@ -1159,32 +1147,15 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
if (!line)
return -1;

- while (line_len != 0 && isspace(line[line_len - 1]))
- line[--line_len] = '\0';
-
- c = strchr(line, '\n');
- if (c)
- *c = 0;
-
- line_ip = -1;
- parsed_line = line;
+ parsed_line = rtrim(line);

/* /filename:linenr ? Save line number and ignore. */
- if (regexec(&file_lineno, line, 2, match, 0) == 0) {
- *line_nr = atoi(line + match[1].rm_so);
+ if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
+ *line_nr = atoi(parsed_line + match[1].rm_so);
return 0;
}

- /*
- * Strip leading spaces:
- */
- tmp = line;
- while (*tmp) {
- if (*tmp != ' ')
- break;
- tmp++;
- }
-
+ tmp = ltrim(parsed_line);
if (*tmp) {
/*
* Parse hexa addresses followed by ':'
--
2.7.4

2017-04-07 14:24:47

[permalink] [raw]

Subject: [PATCH 2/5] perf stat: Refactor the code to strip csv output with ltrim()

To strip csv output, use ltrim() instead of
just while loop and isspace() at print_metric_{only}_csv().

Cc: Andi Kleen <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-stat.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2158ea1..868e086a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -875,10 +875,7 @@ static void print_metric_csv(void *ctx,
return;
}
snprintf(buf, sizeof(buf), fmt, val);
- vals = buf;
- while (isspace(*vals))
- vals++;
- ends = vals;
+ ends = vals = ltrim(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
*ends = 0;
@@ -950,10 +947,7 @@ static void print_metric_only_csv(void *ctx, const char *color __maybe_unused,
return;
unit = fixunit(tbuf, os->evsel, unit);
snprintf(buf, sizeof buf, fmt, val);
- vals = buf;
- while (isspace(*vals))
- vals++;
- ends = vals;
+ ends = vals = ltrim(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
*ends = 0;
--
2.7.4

2017-04-07 14:25:07

[permalink] [raw]

Subject: [PATCH 3/5] perf ui browser: Refactor the code to parse color configs with ltrim()

When parsing {fore, back} ground color configs,
use ltrim() instead of just while loop and isspace().

Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/ui/browser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 3eb3edb..9e47ccb 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -579,7 +579,7 @@ static int ui_browser__color_config(const char *var, const char *value,
break;

*bg = '\0';
- while (isspace(*++bg));
+ bg = ltrim(++bg);
ui_browser__colorsets[i].bg = bg;
ui_browser__colorsets[i].fg = fg;
return 0;
--
2.7.4

2017-04-07 14:25:51

[permalink] [raw]

Subject: [PATCH 4/5] perf pmu: Refactor wordwrap() with ltrim()

Cc: Andi Kleen <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/util/pmu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 362051e..11c7525 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1148,8 +1148,7 @@ static void wordwrap(char *s, int start, int max, int corr)
break;
s += wlen;
column += n;
- while (isspace(*s))
- s++;
+ s = ltrim(s);
}
}

--
2.7.4

2017-04-07 15:01:44

[permalink] [raw]

Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

Em Fri, Apr 07, 2017 at 11:24:17PM +0900, Taeung Song escreveu:
> When parsing disassemble lines,
> use ltrim() and rtrim() to strip them,
> not using just while loop and isspace().
>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/util/annotate.c | 49 ++++++++++------------------------------------
> 1 file changed, 10 insertions(+), 39 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index a37032b..1b4f17b 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -379,9 +379,7 @@ static int mov__parse(struct arch *arch, struct ins_operands *ops, struct map *m
> if (comment == NULL)
> return 0;
>
> - while (comment[0] != '\0' && isspace(comment[0]))
> - ++comment;
> -
> + comment = ltrim(comment);
> comment__symbol(ops->source.raw, comment, &ops->source.addr, &ops->source.name);
> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>
> @@ -426,9 +424,7 @@ static int dec__parse(struct arch *arch __maybe_unused, struct ins_operands *ops
> if (comment == NULL)
> return 0;
>
> - while (comment[0] != '\0' && isspace(comment[0]))
> - ++comment;
> -
> + comment = ltrim(comment);
> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>
> return 0;
> @@ -777,10 +773,7 @@ static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, str
>
> static int disasm_line__parse(char *line, const char **namep, char **rawp)
> {
> - char *name = line, tmp;
> -
> - while (isspace(name[0]))
> - ++name;
> + char tmp, *name = ltrim(line);
>
> if (name[0] == '\0')
> return -1;
> @@ -798,12 +791,7 @@ static int disasm_line__parse(char *line, const char **namep, char **rawp)
> goto out_free_name;
>
> (*rawp)[0] = tmp;
> -
> - if ((*rawp)[0] != '\0') {
> - (*rawp)++;
> - while (isspace((*rawp)[0]))
> - ++(*rawp);
> - }
> + *rawp = ltrim(*rawp);
>
> return 0;
>
> @@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> {
> struct annotation *notes = symbol__annotation(sym);
> struct disasm_line *dl;
> - char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
> + char *line = NULL, *parsed_line, *tmp, *tmp2;
> size_t line_len;
> - s64 line_ip, offset = -1;
> + s64 line_ip = -1, offset = -1;

Try to avoid doing these unrelated changes, i.e. moving the setting of
line_ip to -1 from down below to here.

It is unrelated to what you're doing here, i.e. using ltrim/rtrim, and
requires looking at the code to see if this can be done, as I don't know
if line_ip is set to something else in-between... I am removing this,
leaving the patch just for rtrim/ltrim.

> regmatch_t match[2];
>
> if (getline(&line, &line_len, file) < 0)
> @@ -1159,32 +1147,15 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> if (!line)
> return -1;
>
> - while (line_len != 0 && isspace(line[line_len - 1]))
> - line[--line_len] = '\0';
> -
> - c = strchr(line, '\n');
> - if (c)
> - *c = 0;
> -
> - line_ip = -1;
> - parsed_line = line;
> + parsed_line = rtrim(line);
>
> /* /filename:linenr ? Save line number and ignore. */
> - if (regexec(&file_lineno, line, 2, match, 0) == 0) {
> - *line_nr = atoi(line + match[1].rm_so);
> + if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
> + *line_nr = atoi(parsed_line + match[1].rm_so);
> return 0;
> }
>
> - /*
> - * Strip leading spaces:
> - */
> - tmp = line;
> - while (*tmp) {
> - if (*tmp != ' ')
> - break;
> - tmp++;
> - }
> -
> + tmp = ltrim(parsed_line);
> if (*tmp) {
> /*
> * Parse hexa addresses followed by ':'
> --
> 2.7.4

2017-04-07 15:04:15

[permalink] [raw]

Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

Em Fri, Apr 07, 2017 at 11:24:17PM +0900, Taeung Song escreveu:
> When parsing disassemble lines,
> use ltrim() and rtrim() to strip them,
> not using just while loop and isspace().
>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/util/annotate.c | 49 ++++++++++------------------------------------
> 1 file changed, 10 insertions(+), 39 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index a37032b..1b4f17b 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -379,9 +379,7 @@ static int mov__parse(struct arch *arch, struct ins_operands *ops, struct map *m
> if (comment == NULL)
> return 0;
>
> - while (comment[0] != '\0' && isspace(comment[0]))
> - ++comment;
> -
> + comment = ltrim(comment);
> comment__symbol(ops->source.raw, comment, &ops->source.addr, &ops->source.name);
> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>
> @@ -426,9 +424,7 @@ static int dec__parse(struct arch *arch __maybe_unused, struct ins_operands *ops
> if (comment == NULL)
> return 0;
>
> - while (comment[0] != '\0' && isspace(comment[0]))
> - ++comment;
> -
> + comment = ltrim(comment);
> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>
> return 0;
> @@ -777,10 +773,7 @@ static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, str
>
> static int disasm_line__parse(char *line, const char **namep, char **rawp)
> {
> - char *name = line, tmp;
> -
> - while (isspace(name[0]))
> - ++name;
> + char tmp, *name = ltrim(line);
>
> if (name[0] == '\0')
> return -1;
> @@ -798,12 +791,7 @@ static int disasm_line__parse(char *line, const char **namep, char **rawp)
> goto out_free_name;
>
> (*rawp)[0] = tmp;
> -
> - if ((*rawp)[0] != '\0') {
> - (*rawp)++;
> - while (isspace((*rawp)[0]))
> - ++(*rawp);
> - }
> + *rawp = ltrim(*rawp);
>
> return 0;
>
> @@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> {
> struct annotation *notes = symbol__annotation(sym);
> struct disasm_line *dl;
> - char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
> + char *line = NULL, *parsed_line, *tmp, *tmp2;
> size_t line_len;
> - s64 line_ip, offset = -1;
> + s64 line_ip = -1, offset = -1;
> regmatch_t match[2];
>
> if (getline(&line, &line_len, file) < 0)
> @@ -1159,32 +1147,15 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> if (!line)
> return -1;
>
> - while (line_len != 0 && isspace(line[line_len - 1]))
> - line[--line_len] = '\0';
> -
> - c = strchr(line, '\n');
> - if (c)
> - *c = 0;
> -
> - line_ip = -1;
> - parsed_line = line;
> + parsed_line = rtrim(line);
>
> /* /filename:linenr ? Save line number and ignore. */
> - if (regexec(&file_lineno, line, 2, match, 0) == 0) {
> - *line_nr = atoi(line + match[1].rm_so);
> + if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
> + *line_nr = atoi(parsed_line + match[1].rm_so);

Both should be the same, so this is further noise in the patch, please
refrain from doing extra things in a patch. If you feel this is really
needed, do it in _another_ patch, with proper explanation.

This is a trivial case, but you need to be consistent, avoiding to lump
together unrelated stuff in the same patch.

> return 0;
> }
>
> - /*
> - * Strip leading spaces:
> - */
> - tmp = line;
> - while (*tmp) {
> - if (*tmp != ' ')
> - break;
> - tmp++;
> - }
> -
> + tmp = ltrim(parsed_line);
> if (*tmp) {
> /*
> * Parse hexa addresses followed by ':'
> --
> 2.7.4

2017-04-07 15:05:07

[permalink] [raw]

Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

Em Fri, Apr 07, 2017 at 12:01:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Apr 07, 2017 at 11:24:17PM +0900, Taeung Song escreveu:
> > @@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> > {
> > struct annotation *notes = symbol__annotation(sym);
> > struct disasm_line *dl;
> > - char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
> > + char *line = NULL, *parsed_line, *tmp, *tmp2;
> > size_t line_len;
> > - s64 line_ip, offset = -1;
> > + s64 line_ip = -1, offset = -1;
>
> Try to avoid doing these unrelated changes, i.e. moving the setting of
> line_ip to -1 from down below to here.
>
> It is unrelated to what you're doing here, i.e. using ltrim/rtrim, and
> requires looking at the code to see if this can be done, as I don't know
> if line_ip is set to something else in-between... I am removing this,
> leaving the patch just for rtrim/ltrim.

Ok, please do it yourself, after taking into account the other comment
on this patch, I'm looking at the other patches now.

- Arnaldo

2017-04-07 15:06:15

[permalink] [raw]

Subject: Re: [PATCH 2/5] perf stat: Refactor the code to strip csv output with ltrim()

Em Fri, Apr 07, 2017 at 11:24:18PM +0900, Taeung Song escreveu:
> To strip csv output, use ltrim() instead of
> just while loop and isspace() at print_metric_{only}_csv().

Applied.

> Cc: Andi Kleen <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/builtin-stat.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 2158ea1..868e086a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -875,10 +875,7 @@ static void print_metric_csv(void *ctx,
> return;
> }
> snprintf(buf, sizeof(buf), fmt, val);
> - vals = buf;
> - while (isspace(*vals))
> - vals++;
> - ends = vals;
> + ends = vals = ltrim(buf);
> while (isdigit(*ends) || *ends == '.')
> ends++;
> *ends = 0;
> @@ -950,10 +947,7 @@ static void print_metric_only_csv(void *ctx, const char *color __maybe_unused,
> return;
> unit = fixunit(tbuf, os->evsel, unit);
> snprintf(buf, sizeof buf, fmt, val);
> - vals = buf;
> - while (isspace(*vals))
> - vals++;
> - ends = vals;
> + ends = vals = ltrim(buf);
> while (isdigit(*ends) || *ends == '.')
> ends++;
> *ends = 0;
> --
> 2.7.4

2017-04-07 18:08:39

[permalink] [raw]

Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

On 04/08/2017 12:04 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 07, 2017 at 12:01:30PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Apr 07, 2017 at 11:24:17PM +0900, Taeung Song escreveu:
>>> @@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>>> {
>>> struct annotation *notes = symbol__annotation(sym);
>>> struct disasm_line *dl;
>>> - char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
>>> + char *line = NULL, *parsed_line, *tmp, *tmp2;
>>> size_t line_len;
>>> - s64 line_ip, offset = -1;
>>> + s64 line_ip = -1, offset = -1;
>>
>> Try to avoid doing these unrelated changes, i.e. moving the setting of
>> line_ip to -1 from down below to here.
>>
>> It is unrelated to what you're doing here, i.e. using ltrim/rtrim, and
>> requires looking at the code to see if this can be done, as I don't know
>> if line_ip is set to something else in-between... I am removing this,
>> leaving the patch just for rtrim/ltrim.
>
> Ok, please do it yourself, after taking into account the other comment
> on this patch, I'm looking at the other patches now.
>
> - Arnaldo
>

Thank you for detailed feedback.
I understood. As you said, I'll remove unrelated things in this patch.
And I'll send v2 based on your comment.

Thanks,
Taeung

2017-04-07 23:45:25

[permalink] [raw]

Subject: Re: [PATCH 2/5] perf stat: Refactor the code to strip csv output with ltrim()

On 04/08/2017 12:06 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 07, 2017 at 11:24:18PM +0900, Taeung Song escreveu:
>> To strip csv output, use ltrim() instead of
>> just while loop and isspace() at print_metric_{only}_csv().
>
> Applied.

Thank you!
- Taeung

2017-04-08 00:12:40

[permalink] [raw]

Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()

On 04/08/2017 12:04 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 07, 2017 at 11:24:17PM +0900, Taeung Song escreveu:
>> When parsing disassemble lines,
>> use ltrim() and rtrim() to strip them,
>> not using just while loop and isspace().
>>
>> Cc: Jiri Olsa <[email protected]>
>> Cc: Namhyung Kim <[email protected]>
>> Signed-off-by: Taeung Song <[email protected]>
>> ---
>> tools/perf/util/annotate.c | 49 ++++++++++------------------------------------
>> 1 file changed, 10 insertions(+), 39 deletions(-)
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index a37032b..1b4f17b 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -379,9 +379,7 @@ static int mov__parse(struct arch *arch, struct ins_operands *ops, struct map *m
>> if (comment == NULL)
>> return 0;
>>
>> - while (comment[0] != '\0' && isspace(comment[0]))
>> - ++comment;
>> -
>> + comment = ltrim(comment);
>> comment__symbol(ops->source.raw, comment, &ops->source.addr, &ops->source.name);
>> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>>
>> @@ -426,9 +424,7 @@ static int dec__parse(struct arch *arch __maybe_unused, struct ins_operands *ops
>> if (comment == NULL)
>> return 0;
>>
>> - while (comment[0] != '\0' && isspace(comment[0]))
>> - ++comment;
>> -
>> + comment = ltrim(comment);
>> comment__symbol(ops->target.raw, comment, &ops->target.addr, &ops->target.name);
>>
>> return 0;
>> @@ -777,10 +773,7 @@ static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, str
>>
>> static int disasm_line__parse(char *line, const char **namep, char **rawp)
>> {
>> - char *name = line, tmp;
>> -
>> - while (isspace(name[0]))
>> - ++name;
>> + char tmp, *name = ltrim(line);
>>
>> if (name[0] == '\0')
>> return -1;
>> @@ -798,12 +791,7 @@ static int disasm_line__parse(char *line, const char **namep, char **rawp)
>> goto out_free_name;
>>
>> (*rawp)[0] = tmp;
>> -
>> - if ((*rawp)[0] != '\0') {
>> - (*rawp)++;
>> - while (isspace((*rawp)[0]))
>> - ++(*rawp);
>> - }
>> + *rawp = ltrim(*rawp);
>>
>> return 0;
>>
>> @@ -1148,9 +1136,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>> {
>> struct annotation *notes = symbol__annotation(sym);
>> struct disasm_line *dl;
>> - char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
>> + char *line = NULL, *parsed_line, *tmp, *tmp2;
>> size_t line_len;
>> - s64 line_ip, offset = -1;
>> + s64 line_ip = -1, offset = -1;
>> regmatch_t match[2];
>>
>> if (getline(&line, &line_len, file) < 0)
>> @@ -1159,32 +1147,15 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>> if (!line)
>> return -1;
>>
>> - while (line_len != 0 && isspace(line[line_len - 1]))
>> - line[--line_len] = '\0';
>> -
>> - c = strchr(line, '\n');
>> - if (c)
>> - *c = 0;
>> -
>> - line_ip = -1;
>> - parsed_line = line;
>> + parsed_line = rtrim(line);
>>
>> /* /filename:linenr ? Save line number and ignore. */
>> - if (regexec(&file_lineno, line, 2, match, 0) == 0) {
>> - *line_nr = atoi(line + match[1].rm_so);
>> + if (regexec(&file_lineno, parsed_line, 2, match, 0) == 0) {
>> + *line_nr = atoi(parsed_line + match[1].rm_so);
>
> Both should be the same, so this is further noise in the patch, please
> refrain from doing extra things in a patch. If you feel this is really
> needed, do it in _another_ patch, with proper explanation.
>
> This is a trivial case, but you need to be consistent, avoiding to lump
> together unrelated stuff in the same patch.

Thank you for detailed advice!
will do as you comment.

- Taeung

>
>> return 0;
>> }
>>
>> - /*
>> - * Strip leading spaces:
>> - */
>> - tmp = line;
>> - while (*tmp) {
>> - if (*tmp != ' ')
>> - break;
>> - tmp++;
>> - }
>> -
>> + tmp = ltrim(parsed_line);
>> if (*tmp) {
>> /*
>> * Parse hexa addresses followed by ':'
>> --
>> 2.7.4

2017-04-12 05:39:31

[permalink] [raw]

Subject: [tip:perf/core] perf ui browser: Refactor the code to parse color configs with ltrim()

Commit-ID: e21600fd41106f7a0ca124cec2404b2b3562768d
Gitweb: http://git.kernel.org/tip/e21600fd41106f7a0ca124cec2404b2b3562768d
Author: Taeung Song <[email protected]>
AuthorDate: Fri, 7 Apr 2017 23:24:19 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 11 Apr 2017 08:45:10 -0300

perf ui browser: Refactor the code to parse color configs with ltrim()

When parsing {fore, back} ground color configs, use ltrim() instead of
just while loop and isspace().

Signed-off-by: Taeung Song <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browser.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 3eb3edb..9e47ccb 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -579,7 +579,7 @@ static int ui_browser__color_config(const char *var, const char *value,
break;

*bg = '\0';
- while (isspace(*++bg));
+ bg = ltrim(++bg);
ui_browser__colorsets[i].bg = bg;
ui_browser__colorsets[i].fg = fg;
return 0;

2017-04-12 05:40:03

[permalink] [raw]

Subject: [tip:perf/core] perf pmu: Refactor wordwrap() with ltrim()

Commit-ID: aa4beb10a94358bf2474d1fc9c4ccde34660cc9d
Gitweb: http://git.kernel.org/tip/aa4beb10a94358bf2474d1fc9c4ccde34660cc9d
Author: Taeung Song <[email protected]>
AuthorDate: Fri, 7 Apr 2017 23:24:20 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 11 Apr 2017 08:45:10 -0300

perf pmu: Refactor wordwrap() with ltrim()

Signed-off-by: Taeung Song <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/pmu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 362051e..11c7525 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1148,8 +1148,7 @@ static void wordwrap(char *s, int start, int max, int corr)
break;
s += wlen;
column += n;
- while (isspace(*s))
- s++;
+ s = ltrim(s);
}
}

2017-04-12 05:40:09

[permalink] [raw]

Subject: [tip:perf/core] perf stat: Refactor the code to strip csv output with ltrim()

Commit-ID: b07c40df1f4e6f937271921cb116d570bb9c4a31
Gitweb: http://git.kernel.org/tip/b07c40df1f4e6f937271921cb116d570bb9c4a31
Author: Taeung Song <[email protected]>
AuthorDate: Fri, 7 Apr 2017 23:24:18 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 11 Apr 2017 08:45:10 -0300

perf stat: Refactor the code to strip csv output with ltrim()

To strip csv output, use ltrim() instead of just while loop and
isspace() at print_metric_{only}_csv().

Signed-off-by: Taeung Song <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-stat.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2158ea1..868e086a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -875,10 +875,7 @@ static void print_metric_csv(void *ctx,
return;
}
snprintf(buf, sizeof(buf), fmt, val);
- vals = buf;
- while (isspace(*vals))
- vals++;
- ends = vals;
+ ends = vals = ltrim(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
*ends = 0;
@@ -950,10 +947,7 @@ static void print_metric_only_csv(void *ctx, const char *color __maybe_unused,
return;
unit = fixunit(tbuf, os->evsel, unit);
snprintf(buf, sizeof buf, fmt, val);
- vals = buf;
- while (isspace(*vals))
- vals++;
- ends = vals;
+ ends = vals = ltrim(buf);
while (isdigit(*ends) || *ends == '.')
ends++;
*ends = 0;

2017-04-12 05:40:43