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

Groups > linux.kernel > #1561276 > unrolled thread

[PATCH 1/2] perf diff: Fix segfault on perf diff -o N option

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] perf diff: Fix segfault on perf diff -o N option Namhyung Kim <namhyung@kernel.org> - 2017-01-18 06:20 +0100
    Re: [PATCH 1/2] perf diff: Fix segfault on perf diff -o N option Jiri Olsa <jolsa@redhat.com> - 2017-01-18 19:30 +0100

#1561276 — [PATCH 1/2] perf diff: Fix segfault on perf diff -o N option

The -o/--order option is to select column number to sort a diff result.
It does the job by adding a hpp field at the beginning of the sort list.
But it should not be added to the output field list as it has no
callbacks required by a output field.

During the setup_sorting(), the perf_hpp__setup_output_field() appends
given sort keys to the output field if it's not there already.

Originally it was checked by the fmt->list being non-empty.  But commit
3f931f2c4274 changed it to check the ->equal callback.  Anyway we don't
need to add the pseudo hpp field to the output field list since it won't
be used for output.  So just skip fields if they have no ->color or
 ->entry callbacks.

Fixes: 3f931f2c4274 ("perf hists: Make hpp setup function generic")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/hist.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 37388397b5bc..4ec79b2f9416 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -560,6 +560,10 @@ void perf_hpp__setup_output_field(struct perf_hpp_list *list)
 	perf_hpp_list__for_each_sort_list(list, fmt) {
 		struct perf_hpp_fmt *pos;
 
+		/* skip sort-only fields ("sort_compute" in perf diff) */
+		if (!fmt->entry && !fmt->color)
+			continue;
+
 		perf_hpp_list__for_each_format(list, pos) {
 			if (fmt_equal(fmt, pos))
 				goto next;
-- 
2.11.0

[toc] | [next] | [standalone]

#1562048

On Wed, Jan 18, 2017 at 02:14:56PM +0900, Namhyung Kim wrote:
> The -o/--order option is to select column number to sort a diff result.
> It does the job by adding a hpp field at the beginning of the sort list.
> But it should not be added to the output field list as it has no
> callbacks required by a output field.
> 
> During the setup_sorting(), the perf_hpp__setup_output_field() appends
> given sort keys to the output field if it's not there already.
> 
> Originally it was checked by the fmt->list being non-empty.  But commit
> 3f931f2c4274 changed it to check the ->equal callback.  Anyway we don't
> need to add the pseudo hpp field to the output field list since it won't
> be used for output.  So just skip fields if they have no ->color or
>  ->entry callbacks.
> 
> Fixes: 3f931f2c4274 ("perf hists: Make hpp setup function generic")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/ui/hist.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 37388397b5bc..4ec79b2f9416 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -560,6 +560,10 @@ void perf_hpp__setup_output_field(struct perf_hpp_list *list)
>  	perf_hpp_list__for_each_sort_list(list, fmt) {
>  		struct perf_hpp_fmt *pos;
>  
> +		/* skip sort-only fields ("sort_compute" in perf diff) */
> +		if (!fmt->entry && !fmt->color)
> +			continue;
> +
>  		perf_hpp_list__for_each_format(list, pos) {
>  			if (fmt_equal(fmt, pos))
>  				goto next;
> -- 
> 2.11.0
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web