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 <a...@linux.intel.com>

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/