[igt-dev] [PATCH i-g-t] RFC: gem: fix compiler warnings

[Juha-Pekka Heikkilä]

On Mon, Oct 14, 2019 at 4:37 PM Tvrtko Ursulin
<tvrtko.ursulin at linux.intel.com> wrote:
>
>
> On 14/10/2019 13:09, Juha-Pekka Heikkila wrote:
> > Fix "warning: declaration of ‘e__’ shadows a previous local"
> > complaints from gcc. There are changes in both lib/ and tests/
> > in this patch as changing only one will create build breakage point.
> >
> > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > ---
> >   lib/igt_gt.h                   | 16 ++++++++--------
> >   tests/i915/gem_eio.c           |  6 +++---
> >   tests/i915/gem_exec_latency.c  |  4 ++--
> >   tests/i915/gem_exec_nop.c      |  8 ++++----
> >   tests/i915/gem_exec_schedule.c |  4 ++--
> >   tests/i915/gem_sync.c          | 16 ++++++++--------
> >   6 files changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> > index 73b5002..dcaf969 100644
> > --- a/lib/igt_gt.h
> > +++ b/lib/igt_gt.h
> > @@ -75,16 +75,16 @@ extern const struct intel_execution_engine {
> >   #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))
> > +     for (const struct intel_execution_engine *e__ ## flags__ = intel_execution_engines;\
> > +          e__ ## flags__->name; \
> > +          e__ ## flags__++) \
> > +             for_if (gem_has_ring(fd__, flags__ = e__ ## flags__->exec_id | e__ ## flags__->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))
> > +     for (const struct intel_execution_engine *e__ ## flags__ = intel_execution_engines;\
> > +          e__ ## flags__->name; \
> > +          e__ ## flags__++) \
> > +             for_if (gem_ring_has_physical_engine(fd__, flags__ = e__ ## flags__->exec_id | e__ ## flags__->flags))
> >
> >   bool gem_ring_is_physical_engine(int fd, unsigned int ring);
> >   bool gem_ring_has_physical_engine(int fd, unsigned int ring);
> > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> > index 892f365..1f95bb9 100644
> > --- a/tests/i915/gem_eio.c
> > +++ b/tests/i915/gem_eio.c
> > @@ -454,7 +454,7 @@ static void test_inflight(int fd, unsigned int wait)
> >               gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> >
> >               gem_quiescent_gpu(fd);
> > -             igt_debug("Starting %s on engine '%s'\n", __func__, e__->name);
> > +             igt_debug("Starting %s on engine '%s'\n", __func__, e__engine->name);
>
> This is a bit novel solution and I can't say I like it. Mostly because
> it is churn, while possibly for same amount of churn we could fix it
> properly by passing in the iterator local explicitly.
>

I agree it is churn, of the most obvious type. Reason for this is I
have not touched these gem tests before, to add meaningfully named
iterators I would need to know what is going on in those places where
these defines are used. This is why I opted to just use names
currently in use in those places and blame macros for funny naming :)

/Juha-Pekka