[asterisk-dev] [svn-commits] tilghman: trunk r116694 - in /trunk/include/asterisk: strings.h utils.h

[Tilghman Lesher]

On Thursday 15 May 2008 18:18:43 Russell Bryant wrote:
> SVN commits to the Digium repositories wrote:
> > Author: tilghman
> > Date: Thu May 15 17:05:47 2008
> > New Revision: 116694
> >
> > URL: http://svn.digium.com/view/asterisk?view=rev&rev=116694
> > Log:
> > Add an extra check in ast_strlen_zero, and make ast_assert() not print
> > the file, line, and function name twice.
> > (Closes issue #12650)
> >
> > Modified:
> >     trunk/include/asterisk/strings.h
> >     trunk/include/asterisk/utils.h
> >
> > Modified: trunk/include/asterisk/strings.h
> > URL:
> > http://svn.digium.com/view/asterisk/trunk/include/asterisk/strings.h?view
> >=diff&rev=116694&r1=116693&r2=116694
> > =========================================================================
> >===== --- trunk/include/asterisk/strings.h (original)
> > +++ trunk/include/asterisk/strings.h Thu May 15 17:05:47 2008
> > @@ -29,10 +29,25 @@
> >
> >  /* You may see casts in this header that may seem useless but they
> > ensure this file is C++ clean */
> >
> > +#ifdef AST_DEVMODE
> > +#define ast_strlen_zero(foo)	_ast_strlen_zero(foo, __FILE__,
> > __PRETTY_FUNCTION__, __LINE__) +static force_inline int
> > _ast_strlen_zero(const char *s, const char *file, const char *function,
> > int line) +{
> > +	if (!s || (*s == '\0')) {
> > +		return 1;
> > +	}
> > +	if (!strcmp(s, "(null)")) {
> > +		ast_log(__LOG_WARNING, file, line, function, "Possible programming
> > error: \"(null)\" is not NULL!\n"); +	}
> > +	return 0;
> > +}
> > +
> > +#else
> >  static force_inline int ast_strlen_zero(const char *s)
> >  {
> >  	return (!s || (*s == '\0'));
> >  }
> > +#endif
>
> The entire reason that I added ast_assert(), posted the patch to this
> issue, and referenced this issue in the commit, is because I was proposing
> that ast_assert() should be used for this check, instead of all of this
> work to conditionally define this function, and every other function that
> you want to check arguments to for debugging purposes.
>
> I would like to see this change redone and simplified to use ast_assert().

If only that actually worked the way you hoped that it would:

FRACK!, Failed assertion !strcmp(s, "(null)") (0) at line 40 in 
_ast_strlen_zero 
of /home/tilghman/Asterisk/asterisk-trunk/include/asterisk/strings.h
[May 15 18:43:12] 
ERROR[26580]: /home/tilghman/Asterisk/asterisk-trunk/include/asterisk/strings.h40 
_ast_strlen_zero: FRACK!, Failed assertion !strcmp(s, "(null)") (0)

So instead of telling me WHERE it failed in the code, it tells me that it
failed in include/asterisk/strings.h, which isn't very helpful.  If you could
show me how to make the error message useful, I'll change it.

Oh, and in case you're wondering, I added the opposite check in the code,
because I hadn't gotten the message to show up in my tests.  So the operation
shown is actually the wrong one, just to show that the wrong message got
printed.

-- 
Tilghman