Re: [Libvisual-devel] VisError

[Gustavo S. B. <gu...@gs...>]

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 programmer=
s
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 happen=
ed!
     We could have this function:

        const char *visual_get_error_message( int errcode )
        {
           if ( errcode < 0 ) errcode *=3D -1;
           if ( errcode >=3D visual_error_messages_size ) errcode =3D 0;
           return visual_error_messages[ errcode ];
        }

     And the table as:

        int visual_error_messages_size =3D 3;
        char visual_error_messages[][] =3D {
           "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 =3D 0;

         VisVideo *visual_video_new_r( int *errcode )
         {
                 VisVideo *video;

                 video =3D visual_mem_new0 (VisVideo, 1);
                 if ( video =3D=3D NULL )
                 {
                         *errcode =3D -2;
                         return NULL;
                 }

                 video->screenbuffer =3D NULL;

              /*
               * By default, we suppose an external buffer will be used.
               */
              video->flags =3D 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 =3D visual_mem_new0 (VisVideo, 1);

Great, you allocated nothing, NULL is returned and where is it checked?
Nowhere...


                video->screenbuffer =3D 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 =3D visual_video_new ();

Uncaught error...

                visual_video_set_depth (video, depth);

another...

                visual_video_set_dimension (video, width, height);

even another...

                video->screenbuffer =3D NULL;
                ret =3D visual_video_allocate_buffer (video);

another...

                if (ret < 0) {
                        /*
                         * Restore the flag set by visual_video_new().
                         */
                        video->flags =3D VISUAL_VIDEO_FLAG_EXTERNAL_BUFFE=
R;
                        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 assume=
d
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?

--=20
Gustavo Sverzut Barbieri
------------------------------------------
Engenharia de Computa=E7=E3o 2001 - UNICAMP
Grupo Pr=F3 Software Livre - UNICAMP
Mobile: (19) 9165 8010
Jabber: gsb...@ja...
ICQ: 17249123