Subject: Re: [PATCH 1/5] perf annotate: Refactor the code to parse disassemble
 lines with {l,r}trim()
To: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
References: <1491575061-704-1-git-send-email-treeze.taeung@gmail.com>
 <1491575061-704-2-git-send-email-treeze.taeung@gmail.com>
 <20170407150130.GC2966@kernel.org> <20170407150453.GE2966@kernel.org>
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>
From: Taeung Song <treeze.taeung@gmail.com>
Message-ID: <19132e97-e43b-7a31-c618-c048a1716477@gmail.com>
Date: Sat, 8 Apr 2017 03:08:27 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <20170407150453.GE2966@kernel.org>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1295
Lines: 35


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