linux-kernel - Re: [PATCH] perf tools: Remove cast of non-varadic function to varadic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874n7xxhnm.fsf@canonical.com>
Date:	Thu, 31 Oct 2013 14:19:25 -0700
From:	Michael Hudson-Doyle <michael.hudson@...aro.org>
To:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Cc:	linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...hat.com>,
	Will Deacon <will.deacon@....com>,
	Jean Pihet <jean.pihet@...aro.org>,
	Namhyung Kim <namhyung.kim@....com>
Subject: Re: [PATCH] perf tools: Remove cast of non-varadic function to varadic

Arnaldo Carvalho de Melo <acme@...stprotocols.net> writes:

> Em Thu, Oct 31, 2013 at 12:50:25PM -0700, Michael Hudson-Doyle escreveu:
>> 4fb71074a570 (perf ui/hist: Consolidate hpp helpers) introduced a cast of
>> percent_color_snprintf to a function pointer type with varargs.  Change
>> percent_color_snprintf to be varadic and remove the cast.
>> 
>> The symptom of this was all percentages being reported as 0.00% in perf
>> report --stdio output on armhf.
>
> git bisect + visual inspection, right?

Well annotate + visual inspection yes.  I haven't tested that it was
that commit.

> Anyway, good catch, interesting it caused problems on just one arch,
> or have you tested this in other arches?

I've only observed the problem on the one platform and don't know the
ABIs well enough to know what to expect on other platforms... even on
armhf it's dependent on details of what the compiler decides to do.

Cheers,
mwh

> Namhyung?
>
> - Arnaldo
>  
>> Signed-off-by: Michael Hudson-Doyle <michael.hudson@...aro.org>
>> ---
>>  tools/perf/ui/hist.c    |  2 +-
>>  tools/perf/util/color.c | 11 +++++++++--
>>  tools/perf/util/color.h |  2 +-
>>  3 files changed, 11 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
>> index 0a19328..78f4c92 100644
>> --- a/tools/perf/ui/hist.c
>> +++ b/tools/perf/ui/hist.c
>> @@ -117,7 +117,7 @@ static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
>>  			      struct perf_hpp *hpp, struct hist_entry *he) 	\
>>  {										\
>>  	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
>> -			  (hpp_snprint_fn)percent_color_snprintf, true);	\
>> +			  percent_color_snprintf, true);			\
>>  }
>>  
>>  #define __HPP_ENTRY_PERCENT_FN(_type, _field)					\
>> diff --git a/tools/perf/util/color.c b/tools/perf/util/color.c
>> index 11e46da..66e44a5 100644
>> --- a/tools/perf/util/color.c
>> +++ b/tools/perf/util/color.c
>> @@ -318,8 +318,15 @@ int percent_color_fprintf(FILE *fp, const char *fmt, double percent)
>>  	return r;
>>  }
>>  
>> -int percent_color_snprintf(char *bf, size_t size, const char *fmt, double percent)
>> +int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...)
>>  {
>> -	const char *color = get_percent_color(percent);
>> +	va_list args;
>> +	double percent;
>> +	const char *color;
>> +
>> +	va_start(args, fmt);
>> +	percent = va_arg(args, double);
>> +	va_end(args);
>> +	color = get_percent_color(percent);
>>  	return color_snprintf(bf, size, color, fmt, percent);
>>  }
>> diff --git a/tools/perf/util/color.h b/tools/perf/util/color.h
>> index dea082b..fced384 100644
>> --- a/tools/perf/util/color.h
>> +++ b/tools/perf/util/color.h
>> @@ -39,7 +39,7 @@ int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
>>  int color_snprintf(char *bf, size_t size, const char *color, const char *fmt, ...);
>>  int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
>>  int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
>> -int percent_color_snprintf(char *bf, size_t size, const char *fmt, double percent);
>> +int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...);
>>  int percent_color_fprintf(FILE *fp, const char *fmt, double percent);
>>  const char *get_percent_color(double percent);
>>  
>> -- 
>> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/