From: Dennis S. <sy...@yo...> - 2004-10-05 17:37:23
|
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. Cheers, Dennis |
From: Duilio P. <dp...@fc...> - 2004-10-05 18:24:18
|
> 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. Now what we have too much headers files on libvisual, probably would be a good point to move all headers to an 'include' directory within CVS. Bye, Duilio. |
From: Dennis S. <sy...@yo...> - 2004-10-06 09:45:27
|
On Tue, 2004-10-05 at 15:21 -0300, Duilio Protti wrote: > > 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. > > Now what we have too much headers files on libvisual, probably would be a > good point to move all headers to an 'include' directory within CVS. Do we have too many headers ?, I'm not sure about this, I had this idea before, but on the other hand... Why is it needed ? Having them in one dir keeps the .h, .c pairs nice als close to each other, (That is how I personally feel about it atleast..) Cheers, Dennis |
From: Gustavo S. B. <gu...@gs...> - 2004-10-06 13:36:34
|
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 |
From: Duilio P. <dp...@fc...> - 2004-10-06 17:11:04
|
> 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. Shure, that's the only purpose of visual_log(). For checking function's preconditions, we have visual_log_return_{val_}if_fail(). Still missing an unified framework for the values that this methods will return, which is exactly what you propose. > * 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... visual_mem_new0() never returns NULL, if cannot get it a new chunk of memory, it shows a message an raises an error. > 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... This is true. > visual_video_set_dimension (video, width, height); > > even another... Another one. > 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; > } I think your idea is right. I guess Dennis has not done this yet waiting for 0.1.8, the really, really cleaned up release :-) Bye, Duilio. |
From: Dennis S. <sy...@yo...> - 2004-10-11 22:30:21
|
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? > |
From: Gustavo S. B. <gu...@gs...> - 2004-10-13 14:54:53
|
On Monday 11 October 2004 19:30, Dennis Smit wrote: > 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. Dennis, Talking to a friend he mentioned TLS (Thread Local Storage). We can use that and the error will be local to the thread it happened... all for free (glibc does that). But someone need to look at it... -- Gustavo Sverzut Barbieri --------------------------------------- Engenharia de Computacao 2001 - UNICAMP GPSL - Grupo Pro Software Livre Cell..: +55 (19) 9165 8010 Jabber: gsb...@ja... ICQ#: 17249123 GPG: 0xB640E1A2 @ wwwkeys.pgp.net |
From: Gustavo S. B. <gu...@gs...> - 2004-10-13 14:59:27
|
On Wednesday 13 October 2004 11:54, Gustavo Sverzut Barbieri wrote: > On Monday 11 October 2004 19:30, Dennis Smit wrote: > > 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. > > Dennis, > > Talking to a friend he mentioned TLS (Thread Local Storage). We can use > that and the error will be local to the thread it happened... all for free > (glibc does that). > > But someone need to look at it... Ah, the bad point is: it just work on linux and solaris... other's don't. -- Gustavo Sverzut Barbieri --------------------------------------- Engenharia de Computacao 2001 - UNICAMP GPSL - Grupo Pro Software Livre Cell..: +55 (19) 9165 8010 Jabber: gsb...@ja... ICQ#: 17249123 GPG: 0xB640E1A2 @ wwwkeys.pgp.net |
From: Dennis S. <sy...@yo...> - 2004-10-13 15:21:25
|
On Wed, 2004-10-13 at 11:58 -0300, Gustavo Sverzut Barbieri wrote: > On Wednesday 13 October 2004 11:54, Gustavo Sverzut Barbieri wrote: > > On Monday 11 October 2004 19:30, Dennis Smit wrote: > > > 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. > > > > Dennis, > > > > Talking to a friend he mentioned TLS (Thread Local Storage). We can use > > that and the error will be local to the thread it happened... all for free > > (glibc does that). > > > > But someone need to look at it... > > Ah, the bad point is: it just work on linux and solaris... other's don't. Well I want portability. What do you think about having an extra VisError ** where it's needed, ofcourse far from every function really needs VisError. Glib does it the same way with GError. |
From: Gustavo S. B. <gu...@gs...> - 2004-10-13 15:25:05
|
On Wednesday 13 October 2004 12:20, Dennis Smit wrote: > On Wed, 2004-10-13 at 11:58 -0300, Gustavo Sverzut Barbieri wrote: > > On Wednesday 13 October 2004 11:54, Gustavo Sverzut Barbieri wrote: > > > On Monday 11 October 2004 19:30, Dennis Smit wrote: > > > > 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. > > > > > > Dennis, > > > > > > Talking to a friend he mentioned TLS (Thread Local Storage). We can use > > > that and the error will be local to the thread it happened... all for > > > free (glibc does that). > > > > > > But someone need to look at it... > > > > Ah, the bad point is: it just work on linux and solaris... other's don't. > > Well I want portability. What do you think about having an extra > VisError ** where it's needed, ofcourse far from every function really > needs VisError. Glib does it the same way with GError. It's ok with me. -- Gustavo Sverzut Barbieri --------------------------------------- Engenharia de Computacao 2001 - UNICAMP GPSL - Grupo Pro Software Livre Cell..: +55 (19) 9165 8010 Jabber: gsb...@ja... ICQ#: 17249123 GPG: 0xB640E1A2 @ wwwkeys.pgp.net |