Re: [PATCH 01/10] perf, tools: Factor out scale conversion code

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Oct 13, 2016 at 02:15:23PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Move the scale factor parsing code to an own function
> to reuse it in an upcoming patch.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/pmu.c | 64 +++++++++++++++++++++++++++------------------------
>  1 file changed, 34 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index b1474dcadfa2..9adae7e7477c 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -94,32 +94,10 @@ static int pmu_format(const char *name, struct list_head *format)
>  	return 0;
>  }
>  
> -static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *name)
> +static double convert_scale(const char *scale, char **end)
>  {
> -	struct stat st;
> -	ssize_t sret;
> -	char scale[128];
> -	int fd, ret = -1;
> -	char path[PATH_MAX];
>  	char *lc;
> -
> -	snprintf(path, PATH_MAX, "%s/%s.scale", dir, name);
> -
> -	fd = open(path, O_RDONLY);
> -	if (fd == -1)
> -		return -1;
> -
> -	if (fstat(fd, &st) < 0)
> -		goto error;
> -
> -	sret = read(fd, scale, sizeof(scale)-1);
> -	if (sret < 0)
> -		goto error;
> -
> -	if (scale[sret - 1] == '\n')
> -		scale[sret - 1] = '\0';
> -	else
> -		scale[sret] = '\0';
> +	double sval;
>  
>  	/*
>  	 * save current locale
> @@ -132,10 +110,8 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *
>  	 * call below.
>  	 */
>  	lc = strdup(lc);
> -	if (!lc) {
> -		ret = -ENOMEM;
> -		goto error;
> -	}
> +	if (!lc)
> +		return 1.0;

So if we can't convert the scale because of an allocation failure
related to locale issues we silently trow it away and do no scale at
all?

- Arnaldo

>  
>  	/*
>  	 * force to C locale to ensure kernel
> @@ -144,13 +120,41 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *
>  	 */
>  	setlocale(LC_NUMERIC, "C");
>  
> -	alias->scale = strtod(scale, NULL);
> +	sval = strtod(scale, end);
>  
>  	/* restore locale */
>  	setlocale(LC_NUMERIC, lc);
> -
>  	free(lc);
> +	return sval;
> +}
> +
> +static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char *name)
> +{
> +	struct stat st;
> +	ssize_t sret;
> +	char scale[128];
> +	int fd, ret = -1;
> +	char path[PATH_MAX];
> +
> +	snprintf(path, PATH_MAX, "%s/%s.scale", dir, name);
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd == -1)
> +		return -1;
> +
> +	if (fstat(fd, &st) < 0)
> +		goto error;
> +
> +	sret = read(fd, scale, sizeof(scale)-1);
> +	if (sret < 0)
> +		goto error;
> +
> +	if (scale[sret - 1] == '\n')
> +		scale[sret - 1] = '\0';
> +	else
> +		scale[sret] = '\0';
>  
> +	alias->scale = convert_scale(scale, NULL);
>  	ret = 0;
>  error:
>  	close(fd);
> -- 
> 2.5.5