Re: [AD] 4.3 error handling

[Chris <chris.kcat@xxxxxxxxxx>]

On Monday 05 June 2006 07:02, Elias Pschernig wrote:
> I think, for C code, those #defines are really ugly.. at least to me
> they do look ugly. If I wanted to use #2, then I would use C++, not C.

But Allegro doesn't use C++. Since Allegro is going to use C (AFAIK), or at 
least expose a C-only API, it wouldn't be able to properly throw real 
exceptions.

Besides, with the way that code is, you can use any of the three at any time. 
Don't want to use the try/catch defines? Then just call 
al_error_disable_throws() once at the beginning of your program and don't 
think anything of them (just be sure to check for failure return codes). Too 
lazy to do your own error checking? Leave throws enabled and don't worry 
about the try and catch defines.

There's many times I wish I could just execute a string of commands and not 
have to check the return code of each one individually. But I'd still like to 
be able to handle the errors manually myself, without having to use a 
seperate callback function. Something like:

al_error_try {
   audio_driver = al_audio_init_driver(NULL);

   main_voice = al_voice_create(audio_driver, 44100, AL_AUDIO_16_BIT_INT, 
AL_AUDIO_4_CH, 0);
   al_voice_get_long(main_voice, AL_AUDIO_FREQUENCY, &voice_freq);
   al_voice_get_enum(main_voice, AL_AUDIO_CHANNELS, &channels);

   main_mixer = al_mixer_create(voice_freq, AL_AUDIO_32_BIT_FLOAT, channels);

   al_voice_attach_mixer(main_voice, main_mixer);
}
al_error_catch {
   AL_ERROR code = al_get_last_error();
   show_my_error("Error initializing sound!\n%s\n", al_get_error_string());
   if(no_sound_is_fatal)
      al_error_throw(code);
}

Having to check each function in that try block manually and basically call 
the same error-handling code would be very wasteful. And putting those few 
lines into a seperate function to install as a callback temporarilly would 
not be very clean (plus the trouble of being able to save and restore a 
previous callback, if one was installed, and optionally continue from outside 
of the "error zone").

NOTE: The al_get_error_string function I threw in up there isn't in the code I 
posted, but it would be useful to have a human-readable string of the problem 
along with a simple error code.

> And actual C++ users wouldn't benefit from setjmp/longjmp and those
> #defines, since real exceptions should be raised for C++. I'd say, the
> best idea for C++ is a wrapper, where al_sound::init would raise an
> exception.

A C++ wrapper could still raise real exceptions:

al_audio::init()
{
   al_error_try {
      driver = al_audio_init_driver(NULL);
   }
   al_error_catch {
      throw al_get_last_error();
   }
}

That also has the benefit of leaving the real throwing in C++, so you needn't 
have to worry about C++ exceptions through C.

> The TLS error code and NULL return is all that seems needed to me for C.

Then that's all you have to use. Just call al_error_disable_throws() and do it 
the C way.

> For the #1 approach, we could have a flag to al_init, like
> al_init(ABORT_ON_ERROR). And then Allegro would automatically abort on
> error - just like in your code.

My code has the benefit of being able to turn such aborts on and off at will. 
Like, say, if you want to init video without doing specific error checking, 
but then check the errors while initializing sound. It's also transparent, so 
you could put in a single try/catch around the whole initialization section 
without having to change the initialization code itself.

> The #3 approach looks much simpler than the #2 approach to me anyway, so
> I don't think anything is wrong with it :)

Then, again, you're free to use it. :) Nothing's forcing you to have to do it 
the first or second way.

> One idea similar to setjmp/longjmp might be an error callback, something
> like:
>
> void al_set_error_callback(void *(user_error_handler)(AL_ERROR error));

It might be a good idea to use that anyway, for uncaught errors (eg. if an 
error is generated and either throws are disabled, or not within a try 
block). The error handler can then return non-0 to tell Allegro to clean up 
and SIGABRT, or 0 to tell it to keep going. Though it probably shouldn't be 
allowed to keep going if throws are enabled.

> Or they could simply have abort
> in there, then we wouldn't even need an extra flag for it in al_init.

Allegro knows best how to clean itself up (as long as it can keep track of its 
own state, anyway). There should always be a way to tell Allegro to exit 
cleanly /now/, and raise a signal for debuggers.


That said, however, I do see a /small/ problem with my code. You wouldn't be 
able to jump out from the try block (via goto, a caught exception, another 
longjmp, etc) and continue using Allegro code, or else the code will throw to 
the wrong setjmp point later on. Though I don't think it's that big of a deal 
to simply note that you can't do that, if something like this is used. You 
could always throw a custom allegro error in the try block and check for it 
in the catch block, where you can jump out however you want.