[igt-dev] [PATCH i-g-t] lib/i915: Use explicit iterator names in for_each_engine()

[Chris Wilson]

Quoting Andi Shyti (2019-10-26 18:39:23)
> Hi Chris,
> 
> I thought we wanted to keep the warnings as a reminder until we
> removed the legacy foreach :)
> 
> [...]
> 
> > diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> > index 73b5002a0..66088d391 100644
> > --- a/lib/igt_gt.h
> > +++ b/lib/igt_gt.h
> > @@ -72,19 +72,21 @@ extern const struct intel_execution_engine {
> >       unsigned flags;
> >  } intel_execution_engines[];
> >  
> > +#define eb_ring(e) ((e)->exec_id | (e)->flags)
> > +
> >  #define for_if(expr__) if (!(expr__)) {} else
> >  
> > -#define for_each_engine(fd__, flags__) \
> > -     for (const struct intel_execution_engine *e__ = intel_execution_engines;\
> > -          e__->name; \
> > -          e__++) \
> > -             for_if (gem_has_ring(fd__, flags__ = e__->exec_id | e__->flags))
> > -
> > -#define for_each_physical_engine(fd__, flags__) \
> > -     for (const struct intel_execution_engine *e__ = intel_execution_engines;\
> > -          e__->name; \
> > -          e__++) \
> > -             for_if (gem_ring_has_physical_engine(fd__, flags__ = e__->exec_id | e__->flags))
> > +#define for_each_engine(it__, fd__) \
> > +     for (const struct intel_execution_engine *it__ = intel_execution_engines;\
> > +          it__->name; \
> > +          it__++) \
> > +             for_if (gem_has_ring(fd__, eb_ring(it__)))
> > +
> > +#define for_each_physical_engine(it__, fd__) \
> > +     for (const struct intel_execution_engine *it__ = intel_execution_engines;\
> > +          it__->name; \
> > +          it__++) \
> > +             for_if (gem_ring_has_physical_engine(fd__, eb_ring(it__)))
> 
> (we already talked once about this) it makes more sense to me the
> iterator to be declared outside the from loop in order to not keep
> it hidden from the user.
> 
> It's also to be consistent with all the rest of the iterations,
> starting from the 'for()'.

This is one C99 feature I wish we could use in the kernel. That and
auto.
-Chris