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

[Erik Faye-Lund]

On Sat, 2019-05-25 at 15:44 -0700, Rob Clark wrote:
> This ends up embedded in a for loop expression, ie. the C part in an
> for (A;B;C)
> 
> iirc, that means it needs to be a C expr rather than statement.. or
> something roughly like that, I'm too lazy to dig out my C grammar
> 

Can't you just call a static helper function to do the validation?
Function calls are valid expressions...

> BR,
> -R
> 
> 
> On Sat, May 25, 2019 at 3:39 PM Ilia Mirkin <imirkin at alum.mit.edu>
> wrote:
> > Why not just do it in a way that works for everyone? Both the
> > do/while
> > method and the ifdef-based method that I suggested work everywhere.
> > Or
> > is there another reason you prefer to use those statement
> > expressions?
> > 
> > On Sat, May 25, 2019 at 6:21 PM Rob Clark <robdclark at gmail.com>
> > wrote:
> > > 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
> > > > 
> > > > 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev