Unnecessarily hostile assert() checking in @audiorecorder class

If I try the following Octave code I get an instant Abort.

r = audiorecorder (8000, 16, 4)
octave-gui: libinterp/dldfcn/audiodevinfo.cc:1660: void octave::audiorecorder::set_channels(int): Assertion `channels_arg == 1 || channels_arg == 2' failed.
fatal: caught signal Aborted -- stopping myself...
Abort (core dumped)

As @jwe has said, “A good interpreter should never crash on valid input”. There’s nothing wrong with the statement I typed, it just isn’t supported by the code.

It seems to me that 1) the @audiorecorder constructor should have better input validation and 2) the C++ assert statement should be downgraded to a call to error(). This would still flag the issue, but wouldn’t necessarily lose user data in the process.

1 Like

I made that change here: octave: ff6e74a8f7ba

Yes, I agree with that change.

There are probably some places where using assert is the right thing to do. For example, things that really should be impossible and indicate a serious error in the implementation? But even then, maybe we should try to convert those calls to assert to call error instead. It looks like we have about 150 calls to assert in the C++ sources now, plus we have a similar situation with calls to panic (usually invoked as panic_impossible ();) to print a message and abort.

I’d say it’s worth evaluating whether a change is needed, but it’s also a fairly low priority for me unless we find instances that cause actual trouble as you did with audiorecorder.

I’ll create a new thread with this project proposal.