[PATCH perf/urgent] perf tools: Fix the code to strip command name

[PATCH perf/urgent] perf tools: Fix the code to strip command name

[Jiri Olsa]

Recent commit broke command name strip in perf_event__get_comm_ids
function. It replaced left to right search for '\n' with rtrim,
which actually does right to left search. It occasionally caught
earlier '\n' and kept trash in the command name.

Keeping the ltrim, but moving back the left to right '\n' search
instead of the rtrim.

Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Taeung Song <treeze.taeung@gmail.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Link: http://lkml.kernel.org/n/tip-51mt8hxaig74zlu42s3rv0i7@git.kernel.org
---
 tools/perf/util/event.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index cf457ef534da..1a9164a816d9 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -138,8 +138,15 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
 	ppids = strstr(bf, "PPid:");
 
 	if (name) {
+		char *nl;
+
 		name += 5;  /* strlen("Name:") */
-		name = rtrim(ltrim(name));
+		name = ltrim(name);
+
+		nl = strchr(name, '\n');
+		if (nl)
+			*nl = '\0';
+
 		size = strlen(name);
 		if (size >= len)
 			size = len - 1;
-- 
2.9.3

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Taeung Song]

Hi Jiri,

On 04/20/2017 06:24 PM, Jiri Olsa wrote:
> Recent commit broke command name strip in perf_event__get_comm_ids
> function. It replaced left to right search for '\n' with rtrim,
> which actually does right to left search. It occasionally caught
> earlier '\n' and kept trash in the command name.

Sorry for my commit that have failings.

Could I know the command name in the above case ?
The command name can have two '\n' ?


Thanks,
Taeung

>
> Keeping the ltrim, but moving back the left to right '\n' search
> instead of the rtrim.
>
> Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Taeung Song <treeze.taeung@gmail.com>
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-51mt8hxaig74zlu42s3rv0i7@git.kernel.org
> ---
>  tools/perf/util/event.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index cf457ef534da..1a9164a816d9 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -138,8 +138,15 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
>  	ppids = strstr(bf, "PPid:");
>
>  	if (name) {
> +		char *nl;
> +
>  		name += 5;  /* strlen("Name:") */
> -		name = rtrim(ltrim(name));
> +		name = ltrim(name);
> +
> +		nl = strchr(name, '\n');
> +		if (nl)
> +			*nl = '\0';
> +
>  		size = strlen(name);
>  		if (size >= len)
>  			size = len - 1;
>

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Jiri Olsa]

On Thu, Apr 20, 2017 at 07:17:34PM +0900, Taeung Song wrote:
> Hi Jiri,
> 
> On 04/20/2017 06:24 PM, Jiri Olsa wrote:
> > Recent commit broke command name strip in perf_event__get_comm_ids
> > function. It replaced left to right search for '\n' with rtrim,
> > which actually does right to left search. It occasionally caught
> > earlier '\n' and kept trash in the command name.
> 
> Sorry for my commit that have failings.
> 
> Could I know the command name in the above case ?
> The command name can have two '\n' ?

it's the next line in the status file.. parts of the Umask string
and 1 newline

Name:   systemd
Umask:  0000
State:  S (sleeping)
...

I've already posted it in here:
  http://marc.info/?l=linuxppc-embedded&m=149200723316270&w=2

jirka

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Taeung Song]

On 04/20/2017 07:35 PM, Jiri Olsa wrote:
> On Thu, Apr 20, 2017 at 07:17:34PM +0900, Taeung Song wrote:
>> Hi Jiri,
>>
>> On 04/20/2017 06:24 PM, Jiri Olsa wrote:
>>> Recent commit broke command name strip in perf_event__get_comm_ids
>>> function. It replaced left to right search for '\n' with rtrim,
>>> which actually does right to left search. It occasionally caught
>>> earlier '\n' and kept trash in the command name.
>>
>> Sorry for my commit that have failings.
>>
>> Could I know the command name in the above case ?
>> The command name can have two '\n' ?
>
> it's the next line in the status file.. parts of the Umask string
> and 1 newline
>
> Name:   systemd
> Umask:  0000
> State:  S (sleeping)
> ...
>
> I've already posted it in here:
>   http://marc.info/?l=linuxppc-embedded&m=149200723316270&w=2
>
> jirka
>

I understood it.
Sorry for my mistake..

Thanks,
Taeung

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Jiri Olsa]

On Thu, Apr 20, 2017 at 07:42:31PM +0900, Taeung Song wrote:
> 
> 
> On 04/20/2017 07:35 PM, Jiri Olsa wrote:
> > On Thu, Apr 20, 2017 at 07:17:34PM +0900, Taeung Song wrote:
> > > Hi Jiri,
> > > 
> > > On 04/20/2017 06:24 PM, Jiri Olsa wrote:
> > > > Recent commit broke command name strip in perf_event__get_comm_ids
> > > > function. It replaced left to right search for '\n' with rtrim,
> > > > which actually does right to left search. It occasionally caught
> > > > earlier '\n' and kept trash in the command name.
> > > 
> > > Sorry for my commit that have failings.
> > > 
> > > Could I know the command name in the above case ?
> > > The command name can have two '\n' ?
> > 
> > it's the next line in the status file.. parts of the Umask string
> > and 1 newline
> > 
> > Name:   systemd
> > Umask:  0000
> > State:  S (sleeping)
> > ...
> > 
> > I've already posted it in here:
> >   http://marc.info/?l=linuxppc-embedded&m=149200723316270&w=2
> > 
> > jirka
> > 
> 
> I understood it.
> Sorry for my mistake..

no worries.. we do plenty of those ;-)

jirka

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Jiri Olsa]

Arnaldo,
could you please take this one?

thanks,
jirka

On Thu, Apr 20, 2017 at 11:24:30AM +0200, Jiri Olsa wrote:
> Recent commit broke command name strip in perf_event__get_comm_ids
> function. It replaced left to right search for '\n' with rtrim,
> which actually does right to left search. It occasionally caught
> earlier '\n' and kept trash in the command name.
> 
> Keeping the ltrim, but moving back the left to right '\n' search
> instead of the rtrim.
> 
> Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Taeung Song <treeze.taeung@gmail.com>
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-51mt8hxaig74zlu42s3rv0i7@git.kernel.org
> ---
>  tools/perf/util/event.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index cf457ef534da..1a9164a816d9 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -138,8 +138,15 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
>  	ppids = strstr(bf, "PPid:");
>  
>  	if (name) {
> +		char *nl;
> +
>  		name += 5;  /* strlen("Name:") */
> -		name = rtrim(ltrim(name));
> +		name = ltrim(name);
> +
> +		nl = strchr(name, '\n');
> +		if (nl)
> +			*nl = '\0';
> +
>  		size = strlen(name);
>  		if (size >= len)
>  			size = len - 1;
> -- 
> 2.9.3
> 

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Arnaldo Carvalho de Melo]

Em Thu, Apr 20, 2017 at 11:24:30AM +0200, Jiri Olsa escreveu:
> Recent commit broke command name strip in perf_event__get_comm_ids
> function. It replaced left to right search for '\n' with rtrim,
> which actually does right to left search. It occasionally caught
> earlier '\n' and kept trash in the command name.
> 
> Keeping the ltrim, but moving back the left to right '\n' search
> instead of the rtrim.

perf/urgent?
 
> Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")

[acme@jouet linux]$ git tag --contains bdd97ca63faa
perf-core-for-mingo-4.12-20170411
perf-core-for-mingo-4.12-20170413
perf-core-for-mingo-4.12-20170419
[acme@jouet linux]$ 

It is just in tip/perf/core, will put in acme/perf/core and push to Ingo
in my next pull req.

Thanks,

- Arnaldo

> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Taeung Song <treeze.taeung@gmail.com>
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Link: http://lkml.kernel.org/n/tip-51mt8hxaig74zlu42s3rv0i7@git.kernel.org
> ---
>  tools/perf/util/event.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index cf457ef534da..1a9164a816d9 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -138,8 +138,15 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
>  	ppids = strstr(bf, "PPid:");
>  
>  	if (name) {
> +		char *nl;
> +
>  		name += 5;  /* strlen("Name:") */
> -		name = rtrim(ltrim(name));
> +		name = ltrim(name);
> +
> +		nl = strchr(name, '\n');
> +		if (nl)
> +			*nl = '\0';
> +
>  		size = strlen(name);
>  		if (size >= len)
>  			size = len - 1;
> -- 
> 2.9.3

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Jiri Olsa]

On Mon, Apr 24, 2017 at 12:44:50PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Apr 20, 2017 at 11:24:30AM +0200, Jiri Olsa escreveu:
> > Recent commit broke command name strip in perf_event__get_comm_ids
> > function. It replaced left to right search for '\n' with rtrim,
> > which actually does right to left search. It occasionally caught
> > earlier '\n' and kept trash in the command name.
> > 
> > Keeping the ltrim, but moving back the left to right '\n' search
> > instead of the rtrim.
> 
> perf/urgent?
>  
> > Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
> 
> [acme@jouet linux]$ git tag --contains bdd97ca63faa
> perf-core-for-mingo-4.12-20170411
> perf-core-for-mingo-4.12-20170413
> perf-core-for-mingo-4.12-20170419
> [acme@jouet linux]$ 
> 
> It is just in tip/perf/core, will put in acme/perf/core and push to Ingo
> in my next pull req.

sure, I did not check.. just thought it's urgent from time POV ;-)

thanks,
jirka

Re: [PATCH perf/urgent] perf tools: Fix the code to strip command name

[Arnaldo Carvalho de Melo]

Em Mon, Apr 24, 2017 at 06:06:45PM +0200, Jiri Olsa escreveu:
> On Mon, Apr 24, 2017 at 12:44:50PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Apr 20, 2017 at 11:24:30AM +0200, Jiri Olsa escreveu:
> > > Recent commit broke command name strip in perf_event__get_comm_ids
> > > function. It replaced left to right search for '\n' with rtrim,
> > > which actually does right to left search. It occasionally caught
> > > earlier '\n' and kept trash in the command name.
> > > 
> > > Keeping the ltrim, but moving back the left to right '\n' search
> > > instead of the rtrim.
> > 
> > perf/urgent?
> >  
> > > Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
> > 
> > [acme@jouet linux]$ git tag --contains bdd97ca63faa
> > perf-core-for-mingo-4.12-20170411
> > perf-core-for-mingo-4.12-20170413
> > perf-core-for-mingo-4.12-20170419
> > [acme@jouet linux]$ 
> > 
> > It is just in tip/perf/core, will put in acme/perf/core and push to Ingo
> > in my next pull req.
> 
> sure, I did not check.. just thought it's urgent from time POV ;-)

:-)

I took it too literally then, tried to apply it to perf/urgent, it
failed, scratched my head...

Anyway, applied.

- Arnaldo

[tip:perf/core] perf tools: Fix the code to strip command name

[tip-bot for Jiri Olsa]

Commit-ID:  9d43f5e8df6804ae271407500af9062e9278167a
Gitweb:     http://git.kernel.org/tip/9d43f5e8df6804ae271407500af9062e9278167a
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 20 Apr 2017 11:24:30 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 24 Apr 2017 13:43:37 -0300

perf tools: Fix the code to strip command name

Recent commit broke command name strip in perf_event__get_comm_ids
function. It replaced left to right search for '\n' with rtrim, which
actually does right to left search. It occasionally caught earlier '\n'
and kept trash in the command name.

Keeping the ltrim, but moving back the left to right '\n' search
instead of the rtrim.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Taeung Song <treeze.taeung@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Yao Jin <yao.jin@linux.intel.com>
Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
Link: http://lkml.kernel.org/r/20170420092430.29657-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2e829ac..142835c 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -141,8 +141,15 @@ static int perf_event__get_comm_ids(pid_t pid, char *comm, size_t len,
 	ppids = strstr(bf, "PPid:");
 
 	if (name) {
+		char *nl;
+
 		name += 5;  /* strlen("Name:") */
-		name = rtrim(ltrim(name));
+		name = ltrim(name);
+
+		nl = strchr(name, '\n');
+		if (nl)
+			*nl = '\0';
+
 		size = strlen(name);
 		if (size >= len)
 			size = len - 1;