Re: [AD] Error reporting from Allegro functions

[Peter Wang <novalazy@xxxxxxxxxx>]

On Fri, 22 Feb 2013 20:34:56 -0500 (EST), SiegeLord <slabode@xxxxxxxxxx> wrote:
> Allegro currently provides two separate methods of indicating the 
> precise cause of an error (usually a NULL return value). The most 
> common method right now uses the ALLEGRO_WARN/ERROR macros. The other 
> method uses the al_get_errno/al_set_errno functions which are basically 
> unused. Some places, like the memfile addon, use al_set_errno with 
> great regularity, and some places, like the image addon, provide very 
> detailed log messages (for most errors... ironically not for the most 
> common one of non-existent file).
> 
> So my question is... which method is preferred and to what extent? 
> Should every "return NULL;" be accompanied with a logging macro? Should 
> every malloc be accompanied with al_set_errno(ENOMEM); ?

Ideally errno would be set consistently, but it's very hard to get into
the practice.  It's hard enough getting new API functions to not return
void (not to pick on it, but the video addon is a case in point).

> 
> Although the trace macros are attractive, I find them to be problematic 
> as they are only enabled in debug mode. While the windows binaries we 
> supply have the debugging versions included, as far as I can tell 
> neither the Fedora packages nor the eventual Debian packages will have 
> this feature. Perhaps we should move away from these macros? Also 
> non-log error reporting is more attractive from the point of view of 
> wrappers of Allegro.

I think they are orthogonal.  I wouldn't remove tracing at all, it's too
useful.  Maybe it's so useful it should be always enabled (removing the
build time option), at least for messages above a certain level.
How to actually handle trace messages could be a config file option.
In debug modes we would still log by default to allegro.log, but not in
release mode.

> 
> If we do move away from macros, perhaps we should add a 
> al_set_error_string function if al_set_errno is deemed too 
> coarse-grained.

It turns out that it's pretty hard to squeeze "multimedia" error
conditions into the "system"-centric errno E* constants, and the subset
which you can reliably use across platforms is not clear (and small).
A freeform string would therefore be attractive, even though the user
cannot (should not) react to freeform strings.  But nobody ever
complained about missing al_set_errno calls so it's likely not an issue.

Peter