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>
