On 01/03/2017 17:39, Michal Wajdeczko wrote:
Engine related definitions are located in different files
and it is easy to break their cross dependency.
Additionally use GEM_WARN_ON to catch invalid engine index.
v2: compare against array size
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a238304..c1f58b5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -84,11 +84,16 @@ static const struct engine_info {
static int
intel_engine_setup(struct drm_i915_private *dev_priv,
- enum intel_engine_id id)
+ unsigned int id)
{
const struct engine_info *info = &intel_engines[id];
struct intel_engine_cs *engine;
+ BUILD_BUG_ON(ARRAY_SIZE(intel_engines) !=
+ ARRAY_SIZE(dev_priv->engine));
+ if (GEM_WARN_ON(id >= ARRAY_SIZE(intel_engines)))
+ return -EINVAL;
+
GEM_BUG_ON(dev_priv->engine[id]);
engine = kzalloc(sizeof(*engine), GFP_KERNEL);
if (!engine)
I would add asserts for id >= ARRAY_SIZE(intel_engines) and id >=
ARRAY_SIZE(dev_priv->engine). That provides guarantees for no out of
bounds access within this function and should be enough for this function.
The rest sounds just like trouble for now.
Also I am not sure about negative enum, do we elsewhere check for that
class of errors? Is it worth it? Maybe then just cast to unsigned in the
above mentioned asserts?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
