Re: [PATCH 21/57] perf c2c report: Add offset dimension key

[Arnaldo Carvalho de Melo]

Em Thu, Sep 22, 2016 at 05:36:49PM +0200, Jiri Olsa escreveu:
> Adding cacheline offset dimension key support.
> It displays cacheline offset as hex number.
> 
> Link: http://lkml.kernel.org/n/tip-m0424ye98lqveg5nopto8qww@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  tools/perf/builtin-c2c.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 335c0fd30757..7c52481ec36b 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -300,6 +300,32 @@ static int dcacheline_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  	return snprintf(hpp->buf, hpp->size, "%*s", width, hex_str(addr));
>  }
>  
> +static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> +			struct hist_entry *he)
> +{
> +	uint64_t addr = 0;
> +	int width = c2c_width(fmt, hpp, he->hists);
> +
> +	if (he->mem_info)
> +		addr = cl_offset(he->mem_info->daddr.al_addr);
> +
> +	return snprintf(hpp->buf, hpp->size, "%*s", width, hex_str(addr));

Please don't use snprintf when you actually use the return from this
function, see this part of its man page:

"If the output was trunâ cated due to this limit, then the return value
is the number of characters (excluding the terminating null byte) which
would have been written  to the  final  string  if enough space had been
available.  Thus, a return value of size or more means that the output
was truncated.  (See also below under NOTES.)"

That is why we're using scnprintf where we need to use its return value.

I'm checking how you use the return value and changing all to scnprintf
instead, ok?

- Arnaldo


> +}
> +
> +static int64_t
> +offset_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> +	   struct hist_entry *left, struct hist_entry *right)
> +{
> +	uint64_t l = 0, r = 0;
> +
> +	if (left->mem_info)
> +		l = cl_offset(left->mem_info->daddr.addr);
> +	if (right->mem_info)
> +		r = cl_offset(right->mem_info->daddr.addr);
> +
> +	return (int64_t)(r - l);
> +}
> +
>  #define HEADER_LOW(__h)			\
>  	{				\
>  		.line[1] = {		\
> @@ -343,8 +369,17 @@ static struct c2c_dimension dim_dcacheline = {
>  	.width		= 18,
>  };
>  
> +static struct c2c_dimension dim_offset = {
> +	.header		= HEADER_BOTH("Data address", "Offset"),
> +	.name		= "offset",
> +	.cmp		= offset_cmp,
> +	.entry		= offset_entry,
> +	.width		= 18,
> +};
> +
>  static struct c2c_dimension *dimensions[] = {
>  	&dim_dcacheline,
> +	&dim_offset,
>  	NULL,
>  };
>  
> -- 
> 2.7.4