Re: [PATCH] perf, tools: Support spark lines in perf stat v4

[Jiri Olsa]

On Tue, May 27, 2014 at 05:29:30PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>

SNIP

> +/* Print spark lines on outf for numval values in val. */
> +void print_spark(FILE *outf, unsigned long *val, int numval)
> +{
> +	static const char *ticks[NUM_SPARKS] = {
> +		"â",  "â", "â", "â", "â", "â", "â", "â"
> +	};
> +	int i;
> +	unsigned long min = ULONG_MAX, max = 0, f;
> +
> +	for (i = 0; i < numval; i++) {
> +		if (val[i] < min)
> +			min = val[i];
> +		if (val[i] > max)
> +			max = val[i];
> +	}
> +	f = ((max - min) << SPARK_SHIFT) / (NUM_SPARKS - 1);
> +	if (f < 1)
> +		f = 1;
> +	for (i = 0; i < numval; i++)
> +		fputs(ticks[((val[i] - min) << SPARK_SHIFT) / f], outf);

I've got segfault in here on 32 bits, due to above computation overflow.
When I used attached patch, it works as expected.

Also please update this with some boundaries checks before
we access the array.

> +}
> diff --git a/tools/perf/util/spark.h b/tools/perf/util/spark.h
> new file mode 100644
> index 0000000..59b04ad
> --- /dev/null
> +++ b/tools/perf/util/spark.h
> @@ -0,0 +1,4 @@
> +#ifndef SPARK_H
> +#define SPARK_H 1
> +void print_spark(FILE *outf, unsigned long *val, int numval);
> +#endif
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 6506b3d..2e76d9c 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -1,10 +1,16 @@
>  #include <math.h>
> +#include <stdio.h>
>  
>  #include "stat.h"
> +#include "spark.h"
>  
>  void update_stats(struct stats *stats, u64 val)
>  {
>  	double delta;
> +	int n = stats->n;
> +
> +	if (n < NUM_SPARK_VALS)
> +		stats->svals[n] = val;

also I think we need svals to be array of u64 values

>  
>  	stats->n++;
>  	delta = val - stats->mean;
> @@ -61,3 +67,31 @@ double rel_stddev_stats(double stddev, double avg)
>  
>  	return pct;
>  }

thanks,
jirka


---
diff --git a/tools/perf/util/spark.c b/tools/perf/util/spark.c
index ac5b3a5..eb35592 100644
--- a/tools/perf/util/spark.c
+++ b/tools/perf/util/spark.c
@@ -12,7 +12,7 @@ void print_spark(FILE *outf, unsigned long *val, int numval)
 		"â",  "â", "â", "â", "â", "â", "â", "â"
 	};
 	int i;
-	unsigned long min = ULONG_MAX, max = 0, f;
+	unsigned long long min = LLONG_MAX, max = 0, f;
 
 	for (i = 0; i < numval; i++) {
 		if (val[i] < min)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/