From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 13/34] xen/arm: gic: Introduce GIC_SGI_MAX Date: Wed, 26 Mar 2014 09:41:32 +0000 Message-ID: <5332A0CC.2010405@linaro.org> References: <1395766541-23979-1-git-send-email-julien.grall@linaro.org> <1395766541-23979-14-git-send-email-julien.grall@linaro.org> <53320FFC.4060707@linaro.org> <1395824590.29683.24.camel@dagon.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WSkL2-0003LD-Ok for xen-devel@lists.xenproject.org; Wed, 26 Mar 2014 09:41:36 +0000 Received: by mail-la0-f42.google.com with SMTP id ec20so1296787lab.29 for ; Wed, 26 Mar 2014 02:41:34 -0700 (PDT) In-Reply-To: <1395824590.29683.24.camel@dagon.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 26/03/14 09:03, Ian Campbell wrote: > enums and ints are often, for better or worse, interchangeable. That's > why the existing assert is there, to catch people who are inadvertently > doing something like this. (I don't think the cast in Stefano's example > is strictly needed, so a real case is less likely to leap out into your > face during review). It's a shame that the compiler is not able to warn when an int is implicitly cast into an enum. >> I think instead of an ASSERT, sgi & 0xff might better. It won't be >> harmless for the GIC, even debug is turned off. Right now, the >> developper can put the GIC in wrong state if the value is not in this range. > > The whole point of this assert is to help catch programmer mistakes. If > the programmers and review process was perfect then the assert would be > unnecessary. It's valid for the compiler to do some optimization and think this ASSERT is not neccessary. So we don't catch programmer mistakes. If we want to keep the assert for this reason, we will have also to add sgi & 0xff to protect non-debug build and compiler which remove this assert. > Does ASSERT(sgi < GIC_SGI_MAX) not compiler without warnings? I forgot to try this solution. Surprisingly, the compiler is able to compile correctly this code. So I can replace the ASSERT(sgi < 16) with ASSERT(sgi < GIC_SGI_MAX) + BUILD_BUG_ON. Regards, -- Julien Grall