[Mesa-dev] [PATCH] list: add some iterator debug

[Rob Clark]

Is there a convenient #ifdef I can use to guard the list_assert()
macro..  I don't really mind if MSVC can't have this, but would rather
not let it prevent the rest of us from having nice things

BR,
-R

On Sat, May 25, 2019 at 1:23 PM Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> Yeah, that's a GNU extension. It also works in clang but not MSVC which is
> used to build NIR.
>
> On May 25, 2019 13:30:29 Rob Clark <robdclark at gmail.com> wrote:
>
> > On Sat, May 25, 2019 at 11:13 AM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> >>
> >> On Sat, May 25, 2019 at 2:03 PM Rob Clark <robdclark at gmail.com> wrote:
> >> >
> >> > From: Rob Clark <robdclark at chromium.org>
> >> >
> >> > Debugging use of unsafe iterators when you should have used the _safe
> >> > version sucks.  Add some DEBUG build support to catch and assert if
> >> > someone does that.
> >> >
> >> > I didn't update the UPPERCASE verions of the iterators.  They should
> >> > probably be deprecated/removed.
> >> >
> >> > Signed-off-by: Rob Clark <robdclark at chromium.org>
> >> > ---
> >> >  src/util/list.h | 23 ++++++++++++++++++-----
> >> >  1 file changed, 18 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/src/util/list.h b/src/util/list.h
> >> > index 09d1b4cae64..6d89a42b226 100644
> >> > --- a/src/util/list.h
> >> > +++ b/src/util/list.h
> >> > @@ -43,6 +43,13 @@
> >> >  #include <assert.h>
> >> >  #include "c99_compat.h"
> >> >
> >> > +#ifdef DEBUG
> >> > +#  define LIST_DEBUG 1
> >> > +#else
> >> > +#  define LIST_DEBUG 0
> >> > +#endif
> >> > +
> >> > +#define list_assert(cond, msg)  ({ if (LIST_DEBUG) assert((cond) && msg); })
> >>
> >> Not sure if it's worth worrying about, but this style of macro
> >> definition can be dangerous. One might use it as
> >>
> >> if (x) list_assert()
> >> else blah;
> >>
> >> With the macro defined as-is, the "else blah" will get attached to the
> >> if in the macro. I believe the common style is to do do {}while(0) to
> >> avoid such issues (or to use an inline function). Alternatively, just
> >> define it differently for LIST_DEBUG vs not.
> >>
> >
> > I think the ({ ... }) should save the day..
> >
> > (hmm, is that c99 or a gnu thing?  I've it isn't avail on some
> > compilers I guess we should disable list_assert() for those?)
> >
> > BR,
> > -R
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>