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>,
Wang Nan <wangnan0@huawei.com>,
Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim()
Date: Fri, 7 Apr 2017 12:01:30 -0300 [thread overview]
Message-ID: <20170407150130.GC2966@kernel.org> (raw)
In-Reply-To: <1491575061-704-2-git-send-email-treeze.taeung@gmail.com>
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 <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> 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
next prev parent reply other threads:[~2017-04-07 15:01 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-07 14:24 [PATCH 0/5] Refactoring with ltrim() and rtrim() Taeung Song
2017-04-07 14:24 ` [PATCH 1/5] perf annotate: Refactor the code to parse disassemble lines with {l,r}trim() Taeung Song
2017-04-07 15:01 ` Arnaldo Carvalho de Melo [this message]
2017-04-07 15:04 ` Arnaldo Carvalho de Melo
2017-04-07 18:08 ` Taeung Song
2017-04-07 15:04 ` Arnaldo Carvalho de Melo
2017-04-08 0:12 ` Taeung Song
2017-04-07 14:24 ` [PATCH 2/5] perf stat: Refactor the code to strip csv output with ltrim() Taeung Song
2017-04-07 15:06 ` Arnaldo Carvalho de Melo
2017-04-07 23:45 ` Taeung Song
2017-04-12 5:35 ` [tip:perf/core] " tip-bot for Taeung Song
2017-04-07 14:24 ` [PATCH 3/5] perf ui browser: Refactor the code to parse color configs " Taeung Song
2017-04-12 5:35 ` [tip:perf/core] " tip-bot for Taeung Song
2017-04-07 14:24 ` [PATCH 4/5] perf pmu: Refactor wordwrap() " Taeung Song
2017-04-12 5:36 ` [tip:perf/core] " tip-bot for Taeung Song
2017-04-07 14:24 ` [PATCH 5/5] perf tools: Refactor the code to strip command name with {l,r}trim() Taeung Song
2017-04-12 5:36 ` [tip:perf/core] " tip-bot for Taeung Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170407150130.GC2966@kernel.org \
--to=acme@kernel.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=treeze.taeung@gmail.com \
--cc=wangnan0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).