Re: [PATCH 1/2] perf stat: refactor aggregation code

[Andi Kleen]

On Tue, Feb 12, 2013 at 03:09:27PM +0100, Stephane Eranian wrote:
> -		fmt = "%s%.0f%s%s";
> +		fmt = "%.0f%s%s";
>  	else if (big_num)
> -		fmt = "%s%'18.0f%s%-25s";
> +		fmt = "%'18.0f%s%-25s";
>  	else
> -		fmt = "%s%18.0f%s%-25s";
> +		fmt = "%18.0f%s%-25s";

I realize the code was like this before, but it's better to not 
use variable sprintf formats, as you lose all the compile
time checking of the compiler and mistakes in printf are common. 
Better to duplicate the sprintf.

The rest looks good to me.

Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>

-Andi
--
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/