From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751638AbdBYEfb (ORCPT ); Fri, 24 Feb 2017 23:35:31 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:33538 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751501AbdBYEfa (ORCPT ); Fri, 24 Feb 2017 23:35:30 -0500 Date: Sat, 25 Feb 2017 13:27:34 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , kernel-team@lge.com, Steven Rostedt , Frederic Weisbecker Subject: Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask() Message-ID: <20170225042734.GA12723@danjae.aot.lge.com> References: <20170224011251.14946-1-namhyung@kernel.org> <20170224011251.14946-2-namhyung@kernel.org> <20170224210853.GB15145@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170224210853.GB15145@kernel.org> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote: > 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 > > Cc: Frederic Weisbecker > > Signed-off-by: Namhyung Kim > > --- > > 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... My bad. The argument should be unsigned. The semantics is that it converts a bitmap value into a character (string). > > > +{ > > + 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) > > > + 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? I'd rather change hex_char() instead. How about this? diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c index 6ab8699f0233..061018b42393 100644 --- a/tools/perf/util/cpumap.c +++ b/tools/perf/util/cpumap.c @@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size) return ret; } -static char hex_char(char val) +static char hex_char(unsigned char val) { - if (0 <= val && val <= 9) + if (val < 10) return val + '0'; - if (10 <= val && val < 16) + if (val < 16) return val - 10 + 'a'; return '?'; }