On Sun, Nov 11, 2001 at 07:11:45PM +0100, Richard Guenther wrote:
> I have some serious concerns about the usage of assert()s in the
> alsa library code. As any public API function is guarded with
> asserts() against calls with incorrect arguments (and more
> asserts for other stuff), a call with incorrect arguments of any
> alsa API function will unconditionally _terminate_ the application.
> This is of course strictly discouraged operation of a library.
It's encouraged for libraries that come with source code because it
helps you to find errors in your program.
> Also, assert() is _not_ designed for being used as checks against
> user errors, but to assure _internal_ consistency - so its correct
> to guard not-exported functions with asserts(), if and _only_ if
> these asserts are not supposed to trigger and if they do, its
> a real _bug_ in the library implementation.
If the library is used in a larger program, it is a _programming_ error
if the program calls a library function with improper arguments.
> This way a compile with -DNDEBUG is possible without crashing within
> alsalib on application/user errors.
>
> Note that with the current use of assert()s error checking is not
> possible from the application, as f.i. a bad device string entered
> by the user does simply terminate the application (if started from
> X even without any clue what happened).
I disagree. assert() is used against _programming_ errors. Supplying an
invalid argument is a programming error of the calling function, and
therefore it is completely valid for the library to trigger an
assert(). Let the program dump core, run "gdb program core", get a
backtrace, and you know immediately what went wrong and why.
It's much better to immediately trigger the assertion so the program
crashes exactly at the point where it _really_ went wrong instead of
silently ignoring the error an leaving the calling program in the
assumption that everything is still OK. If the program thinks it's OK
it will only crash in a completely unrelated piece of code
> So the following steps need to be done before releasing 1.0:
> - remove all assert()s from exported API functions, instead
> replace them with usual checks and return appropriate errors
> instead (and document this). F.i.
Only change the assertions into errors in a couple of initialisation
functions, certainly not in all functions.
> const char *snd_pcm_name(snd_pcm_t *pcm)
> {
> assert(pcm);
> return pcm->name;
> }
> should become
> const char *snd_pcm_name(snd_pcm_t *pcm)
> {
> if (!pcm)
> return NULL;
> return pcm->name;
> }
This is an excellent example of a function that shouldn't be changed.
If it is called with a NULL pcm parameter, it's a programming error of
the calling function.
Erik
--
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Faculty
of Information Technology and Systems, Delft University of Technology,
PO BOX 5031, 2600 GA Delft, The Netherlands Phone: +31-15-2783635
Fax: +31-15-2781843 Email: [EMAIL PROTECTED]
WWW: http://www-ict.its.tudelft.nl/~erik/
_______________________________________________
Alsa-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/alsa-devel
