Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1627233 > unrolled thread

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

Started byJiri Olsa <jolsa@kernel.org>
First post2017-04-20 11:30 +0200
Last post2017-04-24 23:30 +0200
Articles 10 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH perf/urgent] perf tools: Fix the code to strip command name Jiri Olsa <jolsa@kernel.org> - 2017-04-20 11:30 +0200
    Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Taeung Song <treeze.taeung@gmail.com> - 2017-04-20 12:20 +0200
      Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Jiri Olsa <jolsa@redhat.com> - 2017-04-20 12:40 +0200
        Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Taeung Song <treeze.taeung@gmail.com> - 2017-04-20 12:50 +0200
          Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Jiri Olsa <jolsa@redhat.com> - 2017-04-20 12:50 +0200
    Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Jiri Olsa <jolsa@redhat.com> - 2017-04-24 13:40 +0200
    Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-24 17:50 +0200
      Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Jiri Olsa <jolsa@redhat.com> - 2017-04-24 18:10 +0200
        Re: [PATCH perf/urgent] perf tools: Fix the code to strip command  name Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-24 18:30 +0200
    [tip:perf/core] perf tools: Fix the code to strip command name tip-bot for Jiri Olsa <tipbot@zytor.com> - 2017-04-24 23:30 +0200

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

FromJiri Olsa <jolsa@kernel.org>
Date2017-04-20 11:30 +0200
Subject[PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tygzw-7a4-7@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


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

FromTaeung Song <treeze.taeung@gmail.com>
Date2017-04-20 12:20 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tyhlT-7Gn-1@gated-at.bofh.it>
In reply to#1627233
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;
>

[toc] | [prev] | [next] | [standalone]


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

FromJiri Olsa <jolsa@redhat.com>
Date2017-04-20 12:40 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tyhFf-7N1-13@gated-at.bofh.it>
In reply to#1627288
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

[toc] | [prev] | [next] | [standalone]


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

FromTaeung Song <treeze.taeung@gmail.com>
Date2017-04-20 12:50 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tyhOV-7Qv-1@gated-at.bofh.it>
In reply to#1627301

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

[toc] | [prev] | [next] | [standalone]


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

FromJiri Olsa <jolsa@redhat.com>
Date2017-04-20 12:50 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tyhOV-7Qv-7@gated-at.bofh.it>
In reply to#1627304
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

[toc] | [prev] | [next] | [standalone]


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

FromJiri Olsa <jolsa@redhat.com>
Date2017-04-24 13:40 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tzKvw-6rB-5@gated-at.bofh.it>
In reply to#1627233
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
> 

[toc] | [prev] | [next] | [standalone]


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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-04-24 17:50 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tzOpr-mK-7@gated-at.bofh.it>
In reply to#1627233
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

[toc] | [prev] | [next] | [standalone]


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

FromJiri Olsa <jolsa@redhat.com>
Date2017-04-24 18:10 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tzOIO-JE-15@gated-at.bofh.it>
In reply to#1629687
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

[toc] | [prev] | [next] | [standalone]


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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-04-24 18:30 +0200
SubjectRe: [PATCH perf/urgent] perf tools: Fix the code to strip command name
Message-ID<tzP2a-Rd-23@gated-at.bofh.it>
In reply to#1629723
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

[toc] | [prev] | [next] | [standalone]


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

Fromtip-bot for Jiri Olsa <tipbot@zytor.com>
Date2017-04-24 23:30 +0200
Subject[tip:perf/core] perf tools: Fix the code to strip command name
Message-ID<tzTIt-3OV-7@gated-at.bofh.it>
In reply to#1627233
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;

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web