[pulseaudio-discuss] asserts active by default

[Jan-Benedict Glaw]

On Tue, 2008-09-02 12:04:19 +0100, Colin Guthrie <gmane at colin.guthr.ie> wrote:
> rdiezmail-pulseaudio at yahoo.de wrote:
> > [2nd try, the first message went apparently lost]
> > 
> > I have noticed that the assert() calls inside the libpulse client
> > library are enabled by default. I would expect asserts to be disabled
> > in release builds, via the NDEBUG macro or similar compile-time
> > switch.
> > 
> > What's the common practice for asserts in the OSS world in this kind
> > of libraries?
> > 
> > I've seen in other projects a "--disable-assert" argument to the
> > ./configure script. Would that (or --enable-assert) be an option to
> > consider?
> 
> The asserts indicate when an invalid use of the API is encountered (or a 
> bug of course!). When an assert happens it indicates an error condition 
> and in pulse, essentially protect against buffer overflows and other 
> such conditions.

You're mixing up two things here.

Invalid API usage should be signaled with a NULL return, or -1, or
whatever makes sense. The same is true for recoverable internal
errors, which additionally could be asserted on in case of NDEBUG not
being set.

But pulse behaves different here. A lot of circumstances where
everybody and her turtle would just return an error, pulse will hit an
assertion.

> If the asserts were disabled, then a significant rework in terms of 
> error handling would need to be performed. If a catchable error occurs 
> in pulse, an assert is not used.

Right, because error handling and assertions were mixed up. Error
handling is for errors that could expectedly happen (eg. malloc()
returns a NULL pointer, user requests an operation that's invalid in
this context, ...)

Assertions should be put on conditions that a user should *never* be
able to provoke in *any* circumstance, like if we *know* we
initialized an internal variable to 1 upon open, we *know* the user
never closed a handle (where it would be set to 0) and we find out
that this given variable is 0, while the handle was never being
closed.

> I'm not really sure of the value in disabling asserts. If they are being 
> hit, then this just means there are bugs to be fixed!

It'll provoke undefined behaviour due to now missing error handling.

> But perhaps I don't get something here, so will wait for a more 
> authoritative answer from Lennart on this :)

I'm quite a happy pulse user, but I found out about this long ago.
ISTR that I even wrote an email at that time, but I'm not sure. I also
seem to remember that Lennart considered audio output unimportant
enough to simply let it run into an assertion instead of clobbering
the code with overly complete error handling.

But I may be wrong here, too :)

MfG, JBG

-- 
      Jan-Benedict Glaw      jbglaw at lug-owl.de              +49-172-7608481
Signature of:                 Gib Dein Bestes. Dann übertriff Dich selbst!
the second  :
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20080902/4c5b3a6e/attachment.pgp>