From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751665AbdJXHix (ORCPT ); Tue, 24 Oct 2017 03:38:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55062 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751430AbdJXHiw (ORCPT ); Tue, 24 Oct 2017 03:38:52 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9452D883BE Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jolsa@redhat.com Date: Tue, 24 Oct 2017 09:38:49 +0200 From: Jiri Olsa To: Andi Kleen Cc: acme@kernel.org, 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: <20171024073849.GA31772@krava> References: <20171020202755.21410-1-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171020202755.21410-1-andi@firstfloor.org> User-Agent: Mutt/1.9.1 (2017-09-22) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 24 Oct 2017 07:38:52 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 >