Re: [Libvisual-devel] VisError

[Dennis S. <sy...@yo...>]

Alright, it took me a while, but I've put some thought into this.

1. There won't be special reentrant functions. 

Why ?, Because when we implement an advanced error report system
we will do it reentrant by design, everywhere. If we go this road
I'd like to do it the following:

Last function argument is of type VisError or something.
NULL can always be passed.

It's no problem if the core breaks, Rather do it good once, than
work around half solution, or providing multiple broken interfaces.

I personally totally go for the last argument is VisError * solution

What do you guys think ?

I kinda formulated my start sentence as if this is final, it's ofcourse
not, so if someone wants to discuss this, please go ahead.

Cheers,
Dennis



On Wed, 2004-10-06 at 10:34 -0300, Gustavo Sverzut Barbieri wrote:
> Dennis Smit said:
> > Added the VisError subsystem to CVS. It consists out of:
> >
> > visual_error_raise
> > visual_error_set_handler
> >
> > I've put these in lv_error.c, lv_error.h because at a
> > later stage we might want to add more error handling
> > stuff, thus having it in it's own files is better.
> >
> > Thanks for the suggestion regarding the error handler
> > Duilio.
> 
> 
> I'm converting the C API to Python, but I want to keep thing OO using
> classes and errors via Exceptions... but I found out that I cannot really
> check what error occurred and looking at the code, many errors are not
> caught.
> 
> IMHO these visual_log() serves just for debug, but not for the programmers
> and methods to enable programmers to get the error messages should be
> allowed.
> 
> * Enabling programmers to get the error messages/code:
> 
>      The best way IMO is to create a table relating error codes to
> messages and functions should return the corresponding error (maybe
> -1 * errorcode).
>      Right now, every error is -1 and there is no way to know what happened!
>      We could have this function:
> 
>         const char *visual_get_error_message( int errcode )
>         {
>            if ( errcode < 0 ) errcode *= -1;
>            if ( errcode >= visual_error_messages_size ) errcode = 0;
>            return visual_error_messages[ errcode ];
>         }
> 
>      And the table as:
> 
>         int visual_error_messages_size = 3;
>         char visual_error_messages[][] = {
>            "Success.",
>            "Unknow Error.",
>            "Can not allocate memory."
>         };
> 
>      This way old functions will return "Unknow Error." until they're
> updated.
> 
>       To handle functions that return pointers we could have them changed
> to _r( ..., int *errcode ) and the version without _r uses the
> global error variable visual_errcode, the "_r" stands for
> "reentrant", just like libc ones. For example:
>          int visual_errcode = 0;
> 
>          VisVideo *visual_video_new_r( int *errcode )
>          {
>                  VisVideo *video;
> 
>                  video = visual_mem_new0 (VisVideo, 1);
>                  if ( video == NULL )
>                  {
>                          *errcode = -2;
>                          return NULL;
>                  }
> 
>                  video->screenbuffer = NULL;
> 
>               /*
>                * By default, we suppose an external buffer will be used.
>                */
>               video->flags = VISUAL_VIDEO_FLAG_EXTERNAL_BUFFER;
> 
>               return video;
>       }
> 
>       VisVideo *visual_video_new()
>       {
>               return visual_video_new( &visual_errcode );
>       }
> 
> 
> 
> 
> * Better Error Handling:
> 
> Take a look at visual_video_new:
> 
>                 video = visual_mem_new0 (VisVideo, 1);
> 
> Great, you allocated nothing, NULL is returned and where is it checked?
> Nowhere...
> 
> 
>                 video->screenbuffer = NULL;
> 
> Segfault! What the user/programmer knows? Nothing.
> 
> What about visual_video_new_with_buffer()?
> 
>         VisVideo *visual_video_new_with_buffer (int width, int height,
> VisVideoDepth depth)
>         {
>                 VisVideo *video;
>                 int ret;
> 
>                 video = visual_video_new ();
> 
> Uncaught error...
> 
>                 visual_video_set_depth (video, depth);
> 
> another...
> 
>                 visual_video_set_dimension (video, width, height);
> 
> even another...
> 
>                 video->screenbuffer = NULL;
>                 ret = visual_video_allocate_buffer (video);
> 
> another...
> 
>                 if (ret < 0) {
>                         /*
>                          * Restore the flag set by visual_video_new().
>                          */
>                         video->flags = VISUAL_VIDEO_FLAG_EXTERNAL_BUFFER;
>                         visual_video_free (video);
>                         return NULL;
>                 }
> 
>                 return video;
>         }
> 
> 
> 
> But since we believe programmers don't do mistake, it's okay... It should
> be used just to enforce our code does the right thing.
>    If we do the right checking on every entry point, things can be assumed
> to be all right, but as visual_video_new_with_buffer() is an entry
> point and we don't check we can't be sure it will work later.
> 
> 
> What do you think?
>