From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752677AbdKHPk5 (ORCPT ); Wed, 8 Nov 2017 10:40:57 -0500 Received: from mail.kernel.org ([198.145.29.99]:54180 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752491AbdKHPk4 (ORCPT ); Wed, 8 Nov 2017 10:40:56 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 70C52218B4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Wed, 8 Nov 2017 12:40:51 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: Andi Kleen , jolsa@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH 1/2] perf, tools: Enable type checking for perf_evsel_config_term types Message-ID: <20171108154051.GB4333@kernel.org> References: <20171020202755.21410-1-andi@firstfloor.org> <20171024073849.GA31772@krava> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024073849.GA31772@krava> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Oct 24, 2017 at 09:38:49AM +0200, Jiri Olsa escreveu: > On Fri, Oct 20, 2017 at 01:27:54PM -0700, Andi Kleen wrote: > > From: Andi Kleen > > > > Use a typed enum for the perf_evsel_config_term type enum. > > This allows gcc to do much stronger type checks, and also > > check for missing case statements. > > > > I removed the unused _MAX member from the number. > > > > It found one missing case. I'm not sure it's a real problem, > > so I just turned it into a BUG_ON for now. > > > > Signed-off-by: Andi Kleen > > --- > > tools/perf/util/evsel.c | 2 ++ > > tools/perf/util/evsel.h | 5 ++--- > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > > index f894893c203d..d1f63b93bf69 100644 > > --- a/tools/perf/util/evsel.c > > +++ b/tools/perf/util/evsel.c > > @@ -775,6 +775,8 @@ static void apply_config_terms(struct perf_evsel *evsel, > > case PERF_EVSEL__CONFIG_TERM_OVERWRITE: > > attr->write_backward = term->val.overwrite ? 1 : 0; > > break; > > + case PERF_EVSEL__CONFIG_TERM_DRV_CFG: > > + BUG_ON(1); > > default: > > break; > > } > > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h > > index db658785d828..6728d8d6e513 100644 > > --- a/tools/perf/util/evsel.h > > +++ b/tools/perf/util/evsel.h > > @@ -37,7 +37,7 @@ struct cgroup_sel; > > * It is allocated within event parsing and attached to > > * perf_evsel::config_terms list head. > > */ > > -enum { > > +enum period_type { > > seems ok, but I'm puzzled with the 'period_type' name > I think it should be more like 'term_type' or something Ok, renamed it to term_type and added your Acked-by, - ARnaldo > jirka > > > > PERF_EVSEL__CONFIG_TERM_PERIOD, > > PERF_EVSEL__CONFIG_TERM_FREQ, > > PERF_EVSEL__CONFIG_TERM_TIME, > > @@ -48,12 +48,11 @@ enum { > > PERF_EVSEL__CONFIG_TERM_OVERWRITE, > > PERF_EVSEL__CONFIG_TERM_DRV_CFG, > > PERF_EVSEL__CONFIG_TERM_BRANCH, > > - PERF_EVSEL__CONFIG_TERM_MAX, > > }; > > > > struct perf_evsel_config_term { > > struct list_head list; > > - int type; > > + enum period_type type; > > union { > > u64 period; > > u64 freq; > > -- > > 2.13.6 > >