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 <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Taeung Song <[email protected]>
Cc: Jin Yao <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
---
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
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 <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Taeung Song <[email protected]>
> Cc: Jin Yao <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> ---
> 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;
>
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
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
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
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 <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Taeung Song <[email protected]>
> Cc: Jin Yao <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> ---
> 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
>
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 <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Taeung Song <[email protected]>
> Cc: Jin Yao <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> ---
> 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
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
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
Commit-ID: 9d43f5e8df6804ae271407500af9062e9278167a
Gitweb: http://git.kernel.org/tip/9d43f5e8df6804ae271407500af9062e9278167a
Author: Jiri Olsa <[email protected]>
AuthorDate: Thu, 20 Apr 2017 11:24:30 +0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Acked-by: Taeung Song <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Yao Jin <[email protected]>
Fixes: bdd97ca63faa ("perf tools: Refactor the code to strip command name with {l,r}trim()")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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;