[Intel-gfx] [PATCH] drm/i915: Use BUILD_BUG_ON to detected missing engine definitions

Michal Wajdeczko michal.wajdeczko at intel.com
Tue Feb 28 14:29:29 UTC 2017


On Tue, Feb 28, 2017 at 02:12:59PM +0000, Chris Wilson wrote:
> On Tue, Feb 28, 2017 at 02:00:50PM +0000, Michal Wajdeczko wrote:
> > Additionally use runtime check to catch invalid engine indices.
> 
> Oh no you don't!

We can skip it, but today there is no way to verify that our enums fits
into [0..I915_NUM_ENGINES) range.

Additionally, you've moved definition of the I915_NUM_ENGINES to
separate file far away from enum intel_engine_id definition.


>  
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index a238304..8df53ae 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -89,6 +89,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
> >  	const struct engine_info *info = &intel_engines[id];
> >  	struct intel_engine_cs *engine;
> >  
> > +	BUILD_BUG_ON(ARRAY_SIZE(intel_engines) != I915_NUM_ENGINES);
> > +	GEM_BUG_ON(id < 0 || id >= I915_NUM_ENGINES);
> 
> Are you sure sparse/smatch won't complain?
> /me too lazy to check the C standard for signedness of an enum without a
> negative value.

enums are int's and as such can be negative ;)
see http://en.cppreference.com/w/c/language/enum

-Michal



More information about the Intel-gfx mailing list