Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

From: Arnaldo Carvalho de Melo
Date: Fri Feb 24 2017 - 16:17:23 EST


Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> The cpu_map__snprint_mask() is to generate string representation of
> cpumask bitmap. For cpu 0 to 11, it'll return "fff".
>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/cpumap.h | 1 +
> 2 files changed, 47 insertions(+)
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 8c7504939113..08540ab2a891 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
> pr_debug("cpumask list: %s\n", buf);
> return ret;
> }
> +
> +static char hex_char(char val)

Why do you use 'char' above and...

> +{
> + if (0 <= val && val <= 9)
> + return val + '0';
> + if (10 <= val && val < 16)
> + return val - 10 + 'a';
> + return '?';
> +}
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
<SNIP>
> + for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> + unsigned char bits = bitmap[cpu / 8];

'unsigned char' here?

Some compilers don't like it, for instance:

19 fedora:24-x-ARC-uClibc: FAIL

CC /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
if (0 <= val && val <= 9)
^
cc1: all warnings being treated as errors

And:

10 debian:experimental-x-arm64: FAIL

CC /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
if (0 <= val && val <= 9)
^~

Are you ok with the patch below?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..405f56ad5c24 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -687,7 +687,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
{
int i, cpu;
char *ptr = buf;
- unsigned char *bitmap;
+ char *bitmap;
int last_cpu = cpu_map__cpu(map, map->nr - 1);

bitmap = zalloc((last_cpu + 7) / 8);
@@ -702,7 +702,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
}

for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
- unsigned char bits = bitmap[cpu / 8];
+ char bits = bitmap[cpu / 8];

if (cpu % 8)
bits >>= 4;


> + if (cpu % 8)
> + bits >>= 4;
> + else
> + bits &= 0xf;
> +
> + *ptr++ = hex_char(bits);
> + if ((cpu % 32) == 0 && cpu > 0)
> + *ptr++ = ',';
> + }
> + *ptr = '\0';
> + free(bitmap);
> +
> + buf[size - 1] = '\0';
> + return ptr - buf;
> +}
> diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
> index 1a0549af8f5c..4d231c6eac0c 100644
> --- a/tools/perf/util/cpumap.h
> +++ b/tools/perf/util/cpumap.h
> @@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
> struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
> struct cpu_map *cpu_map__read(FILE *file);
> size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
> size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
> int cpu_map__get_socket_id(int cpu);
> int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);
> --
> 2.11.1