[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 13/34] xen/arm: gic: Introduce GIC_SGI_MAX





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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.