From: Joerg H. <jo...@de...> - 2004-05-16 14:17:51
|
Hi I've just doing some review especially on error handling when doing malloc and realloc. I think it's not good to declare lot of functions as "void" and in case of an error just do a "return;" in that case you canot detect errors and behave respectivly. I prefer a to replace at least some void-functions with (signed) int with a standard return value of 1; if an error happens, they will return something != 1 (normally < 1). This will probably produce at lot more if(function()) constructs, but I think it's worth. What do you think about that? Joerg -- Fachbegriffe der Informatik (Nr 78): Kinderpornographie im Internet - Schwarzer Peter für den nächsten Wahlkampf Florian Kuehnert |
From: Jan P. <pa...@pi...> - 2005-03-30 09:25:21
|
Hi Lutz and others, I am thinking about reasonable error handling in libexif. Lutz occasionally adds calls of exif_log() when something goes wrong. Various error conditions can be determined by constants like EXIF_LOG_CODE_CORRUPT_DATA. The bad thing is that exif_log() is then followed by return; because most functions return void. I believe it would be much better if the functions returned some completion status, like 0 for success and non-zero values like EXIF_LOG_CODE_CORRUPT_DATA when something goes wrong. Besides corrupted data, it is currently nearly impossible e.g. to see why no libexif data was obtained - it can be missing libExif APP marker, no rights to the JPEG file or even suddently missing JPEG file. The only way how to see what went wrong is actually kind of asynchronous interception of results of exif_log invocations and analysis of the results later ed = exif_data_new_from_file(fileName); if (ed == NULL) { analyze recorded exif_log information to see what failed. Most likely memory allocation problem _somewhere_ return FAILURE; } // ed is not NULL. 3 scenarios possible: // 1) evertyhing went OK // 2) no appropriate APP marker // 3) opening the file failed analyze recorded exif_log information to see if 1) is the case Changing return code from void to some enum type wouldn't cause any binary incompatibility issue - the code would be backward binary compatible. What do you think? This my proposal relates to bug #1054323 where Hans already mentioned this issue. --- Jan |
From: Lutz <lu...@us...> - 2005-03-30 10:48:00
|
On Wed, 2005-03-30 at 11:25 +0200, Jan Patera wrote: > Lutz occasionally adds calls of exif_log() when something goes wrong. > Various error conditions can be determined by constants > like EXIF_LOG_CODE_CORRUPT_DATA.=20 > The bad thing is that exif_log() is then followed by return; > because most functions return void.=20 I wouldn't call this a bad thing. libexif has processed the request as far as possible (even when having encountered non-critical problems like unknown markers and so on). Everything (including errors) has been communicated to the frontend. > I believe it would be > much better if the functions returned some completion status, like > 0 for success and non-zero values like EXIF_LOG_CODE_CORRUPT_DATA when > something goes wrong. It certainly would make programming much easier in some cases. But what do in case of multiple errors (like 2 unknown tags and 1 bad format but otherwise data ok)? I'd rather let frontends scan the log messages for errors. If there are categories missing (AGAINST_SPEC, NOT_ENOUGH_DATA...) we can just add them. > Besides corrupted data, it is currently nearly impossible e.g. > to see why no libexif data was obtained - it can be missing libExif > APP marker, no rights to the JPEG file or even suddently missing > JPEG file. libexif is about reading EXIF data. Handling above problems is not in the scope of libexif (IMHO). In my opinion, exif_data_new_from_file is just a quick&dirty convenience function. It works in most cases, but you'd better: exif_data_new_mm exif_data_log exif_data_load_data > The only way how to see what went wrong is actually kind of > asynchronous interception of results of exif_log invocations > and analysis of the results later Which is not so bad. That way, you've got all information instead of just a code.=20 --=20 Lutz M=FCller <lu...@us...> |
From: Lutz <lu...@us...> - 2005-04-03 17:44:24
|
On Wed, 2005-03-30 at 19:23 +0200, Jan Patera wrote: > should I understand it as you are strictly against completion codes > returned by exif functions? No, not at all. All I wanted to do is to make people aware of the limits of error codes. Let's say we define EXIF_RESULT_OK, EXIF_RESULT_SOMEWHAT_OK and EXIF_RESULT_NOT_OK. What code should libexif return if it encounters an unknown tag or a wrong format on loading? --=20 Lutz M=FCller <lu...@us...> |
From: Jan P. <pa...@pi...> - 2005-04-04 21:27:57
|
Hi, thanks for your contribution to the discussion - I hope more & more people will join us. LibExif is small library although quite complex. I'd like to avoid blowing it up with complicated stacks or tables of error handlers. -- Jan >> Let's say we define EXIF_RESULT_OK, EXIF_RESULT_SOMEWHAT_OK and >> EXIF_RESULT_NOT_OK. What code should libexif return if it encounters >> an unknown tag or a wrong format on loading? > > I don't intend to suggest making things complex, but two options pop to > mind that would clarify this: > > (1) Error handlers. > > Make an exif_set_handler(int error, void (*handler)(int err, char > *msg)) or the like (my definition is just for example). Keep a table of > error handlers. Set the default handler for every error to be the > logging function currently in use. This makes EXIF_RESULT_SOMEWHAT_OK > disambiguous, because by that point a developer will know what has > happened, and it maintains compatibility with the current API, because > by default, everything is still logged with the log handler. > > (2) Error stack. > > Make a simple struct that defines an error condition, and return a > pointer to a stack of them. Yes, this means memory allocation, but it > leaves it up to the developer as to what to do with it. Then it's just = a > matter of exif_errstack_cleanup(errstack *). This, however, leaves the > current logging routines in an awkward limbo, where developers may > develop around the stack, forget logging, and then, sometime down the > road, someone will push for removing it from the API, because > developers can already log things based on the error stack. > > > I think that (1) is probably the best approach as (2) is horribly > ugly, but I'm just throwing ideas out there. Let me know if I'm > horribly off base. > > -- Mike "Dexter" Church > > > ------------------------------------------------------- > SF email is sponsored by - The IT Product Guide > Read honest & candid reviews on hundreds of IT Products from real users= . > Discover which products truly live up to the hype. Start reading now. > http://ads.osdn.com/?ad_id=3D6595&alloc_id=3D14396&op=3Dclick > _______________________________________________ > Libexif-devel mailing list > Lib...@li... > https://lists.sourceforge.net/lists/listinfo/libexif-devel |
From: Lutz <lu...@us...> - 2005-04-04 22:31:54
|
On Mon, 2005-04-04 at 23:24 +0200, Jan Patera wrote: > To the complexity: I was originally thinking about upto 10 status > codes, now my estimate is 10-15. Let's get down to business: Could you assemble this list? We can discuss the prefix (EXIF_RESULT or EXIF_LOG_CODE, whatever) later. DEBUG: - DATA - MESSAGE WARNING: - UNKNOWN_TAG - BAD_FORMAT ERROR: - NO_MEMORY - DATA_AGAINST_SPECIFICATION - TOO_FEW_DATA - NO_EXIF_DATA - BAD_PARAMETER Regards --=20 Lutz M=FCller <lu...@us...> |
From: Lutz <lu...@us...> - 2005-04-04 22:35:03
|
On Tue, 2005-04-05 at 00:15 +0200, Hans Ulrich Niedermann wrote: > BTW, is anybody against me adding gtk-doc build stuff to libexif so > that people with gtk-doc and willing to invest a few CPU cycles can > build API docs? Why not. It's a bit overkill as there are no gtk-objects, signals and so on, but it should be better than nothing. --=20 Lutz M=FCller <lu...@us...> |
From: Hans U. N. <gp...@n-...> - 2005-04-04 23:09:16
|
Lutz M=C3=BCller <lu...@us...> writes: > On Tue, 2005-04-05 at 00:15 +0200, Hans Ulrich Niedermann wrote: >> BTW, is anybody against me adding gtk-doc build stuff to libexif so >> that people with gtk-doc and willing to invest a few CPU cycles can >> build API docs? > > Why not. It's a bit overkill as there are no gtk-objects, signals and so > on, but it should be better than nothing. The alternative would be doxygen. I have to reacquaint me with both, so I'm not biased. I just thought as we were using gtk-doc for libgphoto2, we could use it here as well. Ideally, the comment markup format would be the same and we could use both. Uli |
From: Jan P. <pa...@pi...> - 2005-03-30 17:24:17
|
Hi Lutz, should I understand it as you are strictly against completion codes returned by exif functions? With the current code, many error situations are not propagated. (see also for example bug #1169213). Ambigious completion codes should be commented like in the header file "part of tags may have been successfully parsed. It is upto you (i.e. the programmer) if you discard everything or use partial results of what has been parsed." I am not calling for changed program logic. I am just proposing that instead if (! is_this_correct(x)) { exif_log(blabla); return; } the code should be like if (! is_this_correct(x)) { exif_log(blabla); return EXIF_STATUS_X_IS_WRONG; } It is up the user of the library (I again mean the programmer) if he decides to parse log are use the error codes. As a programmer, I _much_ more prefer the error codes. Which doesn't mean I don't like displaying the log messages. I just want offer the user (or do automatically) just only the appropriat= e actions when something goes as not expected. -- Jan > On Wed, 2005-03-30 at 11:25 +0200, Jan Patera wrote: >> Lutz occasionally adds calls of exif_log() when something goes wrong. >> Various error conditions can be determined by constants >> like EXIF_LOG_CODE_CORRUPT_DATA. >> The bad thing is that exif_log() is then followed by return; >> because most functions return void. > > I wouldn't call this a bad thing. libexif has processed the request as > far as possible (even when having encountered non-critical problems lik= e > unknown markers and so on). Everything (including errors) has been > communicated to the frontend. > >> I believe it would be >> much better if the functions returned some completion status, like 0 >> for success and non-zero values like EXIF_LOG_CODE_CORRUPT_DATA when >> something goes wrong. > > It certainly would make programming much easier in some cases. But what > do in case of multiple errors (like 2 unknown tags and 1 bad format but > otherwise data ok)? I'd rather let frontends scan the log messages for > errors. If there are categories missing (AGAINST_SPEC, > NOT_ENOUGH_DATA...) we can just add them. > >> Besides corrupted data, it is currently nearly impossible e.g. >> to see why no libexif data was obtained - it can be missing libExif >> APP marker, no rights to the JPEG file or even suddently missing JPEG >> file. > > libexif is about reading EXIF data. Handling above problems is not in > the scope of libexif (IMHO). In my opinion, exif_data_new_from_file is > just a quick&dirty convenience function. It works in most cases, but > you'd better: > exif_data_new_mm > exif_data_log > exif_data_load_data > >> The only way how to see what went wrong is actually kind of >> asynchronous interception of results of exif_log invocations >> and analysis of the results later > > Which is not so bad. That way, you've got all information instead of > just a code. > -- > Lutz M=FCller <lu...@us...> |
From: Jan P. <pa...@pi...> - 2005-04-03 18:08:02
|
Hi Lutz, > On Wed, 2005-03-30 at 19:23 +0200, Jan Patera wrote: >> should I understand it as you are strictly against completion codes >> returned by exif functions? > > No, not at all. All I wanted to do is to make people aware of the limit= s > of error codes. > > Let's say we define EXIF_RESULT_OK, EXIF_RESULT_SOMEWHAT_OK and > EXIF_RESULT_NOT_OK. What code should libexif return if it encounters an > unknown tag or a wrong format on loading? EXIF_RESULT_UNKNOWN_TAG and EXIF_RESULT_WRONG_TAG_FORMAT, EXIF_RESULT_WRONG_SOMETHING_FORMAT and EXIF_RESULT_WRONG_SOMETHING_ELSE_FORMAT. Depending on the situation. There would be as many values as needed. The values could be like SCODE/HRESULT in COM and similar interfaces: some bits would indicate success/warning/error and some bits what actually happened. There would be macros for simple checks if (fatal) error or warning occured. // Values are 32 bit values layed out as follows: // // 3 3 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1 1 // 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 // +---+-+-+-----------------------+-------------------------------+ // |Sev| | Facility | Code | // +---+-+-+-----------------------+-------------------------------+ // // where // Sev - is the severity code // 00 - Success // 01 - Informational // 10 - Warning // 11 - Error // Facility - is the facility code // Code - is the facility's status code #define EXIF_ERROR 0xC0000000 #define EXIF_WARN 0x80000000 Examples: #define EXIF_FAILED(x) (((x) & 0xC0000000) =3D=3D EXIF_ERROR) ? TRUE : F= ALSE) #define EXIF_ALMOST_OK(x) (((x) & 0xC0000000) =3D=3D EXIF_WARN) ? TRUE : = FALSE) #define EXIF_NO_MEMORY(x) (((x) & 0xFFFF) =3D=3D EXIF_CODE_NOMEMORY) ? TR= UE : FALSE) // START of internal values #define EXIF_CODE_NOMEMORY 1 #define EXIF_CODE_CANT_OPEN 2 #define EXIF_CODE_NO_DATA 3 #define EXIF_FAC_IFD 0x10000 // IFD parser #define EXIF_FAC_LOADER 0x20000 // JPEG loader #define EXIF_FAC_MNOTE 0x30000 // END of internal values #define EX_MNOTE_NO_MEMORY (EXIF_FAC_MNOTE | EXIF_EXIF_CODE_NOMEMORY) #define EX_LOADER_NO_MEMORY (EXIF_FAC_LOADER | EXIF_CODE_NOMEMORY) #define EX_LOADER_NO_MEMORY_FATAL (EXIF_FAC_LOADER | EXIF_CODE_NOMEMORY | EXIF_ERROR) #define EX_CANNOT_OPEN_FILE (EXIF_FAC_LOADER | EXIF_CODE_CANT_OPEN | EXIF_ERROR) #define EX_WRONG_TAG_FORMAT (EXIF_WARN | EXIF_FAC_IFD | EXIF_CODE_WRONG_FORMAT) #define EX_WRONG_IFD_FORMAT (EXIF_WARN | EXIF_FAC_LOADER | EXIF_CODE_WRONG_FORMAT) #define EX_WRONG_MNOTE_FORMAT (EXIF_WAR | EXIF_FAC_MNOTE | EXIF_CODE_WRONG_FORMAT) #define EX_NO_EXIF_DATA (EXIF_WARN | EXIF_FAC_LOADER | EXIF_CODE_NO_DATA) Amd of course also this one ;-) #define EX_OK 0 All relevant "returns;"'s would be replaced with appropriate "return EX_CANNOT_OPEN_FILE;" or "return EX_WRONG_MNOTE_FORMAT;" Quite powerfull bit-mask macros with mininum overhead in libEXIF (just changed returns) giving lots of information. I hope you've got the idea ;-) How do you (and also others) like it? --- Jan |
From: Poindexter F. <sli...@gm...> - 2005-04-03 22:08:31
|
> Let's say we define EXIF_RESULT_OK, EXIF_RESULT_SOMEWHAT_OK and > EXIF_RESULT_NOT_OK. What code should libexif return if it encounters an > unknown tag or a wrong format on loading? I don't intend to suggest making things complex, but two options pop to mind that would clarify this: (1) Error handlers. Make an exif_set_handler(int error, void (*handler)(int err, char *msg)) or the like (my definition is just for example). Keep a table of error handlers. Set the default handler for every error to be the logging function currently in use. This makes EXIF_RESULT_SOMEWHAT_OK disambiguous, because by that point a developer will know what has happened, and it maintains compatibility with the current API, because by default, everything is still logged with the log handler. (2) Error stack. Make a simple struct that defines an error condition, and return a pointer to a stack of them. Yes, this means memory allocation, but it leaves it up to the developer as to what to do with it. Then it's just a matter of exif_errstack_cleanup(errstack *). This, however, leaves the current logging routines in an awkward limbo, where developers may develop around the stack, forget logging, and then, sometime down the road, someone will push for removing it from the API, because developers can already log things based on the error stack. I think that (1) is probably the best approach as (2) is horribly ugly, but I'm just throwing ideas out there. Let me know if I'm horribly off base. -- Mike "Dexter" Church |
From: Lutz <lu...@us...> - 2005-04-04 05:49:47
|
On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: > EXIF_RESULT_UNKNOWN_TAG and EXIF_RESULT_WRONG_TAG_FORMAT, Do I understand correctly that you wouldn't be able to return both EXIF_RESULT_UNKNOWN_TAG and EXIF_RESULT_WRONG_TAG_FORMAT together? Would you just return EXIF_RESULT_WARN and the first error code occurred? Regards --=20 Lutz M=FCller <lu...@us...> |
From: Jan P. <pa...@pi...> - 2005-04-04 07:52:32
|
> On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: >> EXIF_RESULT_UNKNOWN_TAG and EXIF_RESULT_WRONG_TAG_FORMAT, > > Do I understand correctly that you wouldn't be able to return both > EXIF_RESULT_UNKNOWN_TAG and EXIF_RESULT_WRONG_TAG_FORMAT together? Woul= d > you just return EXIF_RESULT_WARN and the first error code occurred? I would return the first (or last - must be discussed) error of the highest severity of all errors/warnings encountered. Presently, the current code gives up parsing an IFD if an unknown tag is encountered (unless in LIBMNOTE) which is not the case when wrong tag format is encountered (via CF & CC macros). Therefore EXIF_RESULT_UNKNOWN_TAG would be returned regardless of the order. The current code can encounter 2 non-100%OK codes only if they are encountered in two different IFDs. --- Jan |
From: Lutz <lu...@us...> - 2005-04-04 17:09:58
|
>> On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: > I would return the first (or last - must be discussed) error of the > highest severity of all errors/warnings encountered. To summarize, exif-result.h would - define 30+ error/warning codes (that must be discussed) - define a hierarchy of those error codes (that must be discussed) I really doubt that programmers will implement checks for that many error codes and get some useful information out of them. gnome-vfs for example has exactly this problem. Let me propose something different. It is up to the frontends to log messages and handle the queue (like now). Functions would return one or more of the following result codes: EXIF_RESULT_NONE =3D 0, EXIF_RESULT_DEBUG =3D 1 << 0, EXIF_RESULT_WARNING =3D 1 << 1, EXIF_RESULT_ERROR =3D 1 << 2 Frontends would do: res =3D exif_data_something () if (res & EXIF_RESULT_ERROR) { "Errors have occurred..." (show logged errors) if (res & EXIF_RESULT_DEBUG) { "Additional debugging information is available. Click here to show it= ." } } else if (res & EXIF_RESULT_WARNING) { StatusBar =3D "There have been problems executing the request. " (show logged warnings) "Click here to show more info." } That is: Only a couple of result codes. Few log codes. No hierarchy. How about that? Lutz |
From: Jan P. <pa...@pi...> - 2005-04-04 21:30:50
|
>>> On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: >> I would return the first (or last - must be discussed) error of the >> highest severity of all errors/warnings encountered. > > To summarize, exif-result.h would > - define 30+ error/warning codes (that must be discussed) > - define a hierarchy of those error codes (that must be discussed) > > Let me propose something different. It is up to the frontends to log > messages and handle the queue (like now). Functions would return one or > more of the following result codes: > > EXIF_RESULT_NONE =3D 0, > EXIF_RESULT_DEBUG =3D 1 << 0, > EXIF_RESULT_WARNING =3D 1 << 1, > EXIF_RESULT_ERROR =3D 1 << 2 > > That is: Only a couple of result codes. Few log codes. No hierarchy. What about a compromise: Enlarging typedef enum { EXIF_LOG_CODE_NONE, EXIF_LOG_CODE_DEBUG, EXIF_LOG_CODE_NO_MEMORY, EXIF_LOG_CODE_CORRUPT_DATA } ExifLogCode; to about 10 values: The logger will have information in machine readable form and this from all invocations. --- Jan |
From: Hans U. N. <gp...@n-...> - 2005-04-04 20:33:49
|
Lutz M=C3=BCller <lu...@us...> writes: >>> On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: >> I would return the first (or last - must be discussed) error of the >> highest severity of all errors/warnings encountered. > > To summarize, exif-result.h would > - define 30+ error/warning codes (that must be discussed) > - define a hierarchy of those error codes (that must be discussed) Sounds pretty complex to me. > I really doubt that programmers will implement checks for that many error > codes and get some useful information out of them. gnome-vfs for example > has exactly this problem. > Let me propose something different. It is up to the frontends to log > messages and handle the queue (like now). Functions would return one or > more of the following result codes: > > EXIF_RESULT_NONE =3D 0, > EXIF_RESULT_DEBUG =3D 1 << 0, > EXIF_RESULT_WARNING =3D 1 << 1, > EXIF_RESULT_ERROR =3D 1 << 2 > > Frontends would do: > > res =3D exif_data_something () > if (res & EXIF_RESULT_ERROR) { > "Errors have occurred..." (show logged errors) > if (res & EXIF_RESULT_DEBUG) { > "Additional debugging information is available. Click here to show it= ." > } > } else if (res & EXIF_RESULT_WARNING) { > StatusBar =3D "There have been problems executing the request. " > (show logged warnings) > "Click here to show more info." if (res & EXIF_RESULT_DEBUG) { "Additional debugging information is available. Click here to show it." } > } > > That is: Only a couple of result codes. Few log codes. No hierarchy. > > How about that? Simple to implement in libexif. Simple to implement in a frontend, if the log functions are memory leak and buffer overflow safe. However, I see a potential problem: There may be cases when the frontend could react in a "sensible" way to certain kinds of errors, if it knew about the nature of the problem. The frontend needs more information in machine readable format, but that should be information relevant to the frontend. We have to keep in mind what we want to achive: 1. Frondends must know when a call to a libexif function failed. 2. The most simple way to transfer the information is its return code, so it would be really nice if the frontend could do a simple =3D=3D 0 or !=3D NULL comparison to find out whether the call succeeded. It is not coincidence that this is the standard way to do error reporting in C. :-) 3. There must be more detailed information for the user - the stuff in the log messages. These are mostly translated, except perhaps for the lowest level debug info. Considering these three points, I'd suggest classic C library/POSIX return conventions, but with a slightly improved error reporting mechanism than a global "errno" variable (I don't think we want to deal with thread local storage). And the logging already goes a long way towards improvements. Of course, functions returning "int" can easily return a large number of different error codes, but with NULL pointers... I'm not sure ATM how to do something similar here without resorting to a global error function... hmm... a callback function perhaps, like the logging? Uli |
From: Jan P. <pa...@pi...> - 2005-04-04 21:24:18
|
Hi Uli & Lutz, thanks a lot for summarization. Items 1. and 2. are exactly the reasons for my proposal. Translation of log messages makes the current situation evem more messy. To the complexity: I was originally thinking about upto 10 status codes, now my estimate is 10-15. I don't expect 30+ status codes. As there are some fatal errors when LibEXIF gives up (fully or partially - skipped rest of IFD) and some _warnings_ when LibEXIF more or less continues (e.g. skipped {not fully interpreted} just 1 tag), I am proposing structuring of the status codes for those who are interest= ed. I really want to avoid thread local storage, extra allocation, stacks or other (complicated) tasks. We are dealing with many corrupted files and buggy SW around, we need reasonable error reporting. --- Jan > Lutz M=C3=BCller <lu...@us...> writes: > >>>> On Sun, 2005-04-03 at 20:07 +0200, Jan Patera wrote: >>> I would return the first (or last - must be discussed) error of the >>> highest severity of all errors/warnings encountered. >> >> To summarize, exif-result.h would >> - define 30+ error/warning codes (that must be discussed) >> - define a hierarchy of those error codes (that must be discussed) > > Sounds pretty complex to me. > >> I really doubt that programmers will implement checks for that many >> error codes and get some useful information out of them. gnome-vfs for >> example has exactly this problem. > > >> Let me propose something different. It is up to the frontends to log >> messages and handle the queue (like now). Functions would return one >> or more of the following result codes: >> >> EXIF_RESULT_NONE =3D 0, >> EXIF_RESULT_DEBUG =3D 1 << 0, >> EXIF_RESULT_WARNING =3D 1 << 1, >> EXIF_RESULT_ERROR =3D 1 << 2 >> >> Frontends would do: >> >> res =3D exif_data_something () >> if (res & EXIF_RESULT_ERROR) { >> "Errors have occurred..." (show logged errors) >> if (res & EXIF_RESULT_DEBUG) { >> "Additional debugging information is available. Click here to show >> it." >> } >> } else if (res & EXIF_RESULT_WARNING) { >> StatusBar =3D "There have been problems executing the request. " >> (show logged warnings) >> "Click here to show more info." > if (res & EXIF_RESULT_DEBUG) { > "Additional debugging information is available. Click here to show > it." > } >> } >> >> That is: Only a couple of result codes. Few log codes. No hierarchy. >> >> How about that? > > Simple to implement in libexif. > > Simple to implement in a frontend, if the log functions are memory leak > and buffer overflow safe. > > However, I see a potential problem: > > There may be cases when the frontend could react in a "sensible" way to > certain kinds of errors, if it knew about the nature of the > problem. > > The frontend needs more information in machine readable format, but tha= t > should be information relevant to the frontend. > > We have to keep in mind what we want to achive: > > 1. Frondends must know when a call to a libexif function failed. > > 2. The most simple way to transfer the information is its return > code, so it would be really nice if the frontend could do a > simple =3D=3D 0 or !=3D NULL comparison to find out whether the ca= ll > succeeded. > > It is not coincidence that this is the standard way to do error > reporting in C. :-) > > 3. There must be more detailed information for the user - the stuff > in the log messages. These are mostly translated, except perhaps > for the lowest level debug info. > > Considering these three points, I'd suggest classic C library/POSIX > return conventions, but with a slightly improved error reporting > mechanism than a global "errno" variable (I don't think we want to deal > with thread local storage). > > And the logging already goes a long way towards improvements. Of > course, functions returning "int" can easily return a large number of > different error codes, but with NULL pointers... I'm not sure ATM how t= o > do something similar here without resorting to a global error > function... hmm... a callback function perhaps, like the logging? > > Uli |
From: Hans U. N. <gp...@n-...> - 2005-04-04 22:17:15
|
"Jan Patera" <pa...@pi...> writes: > thanks for your contribution to the discussion - I hope more & more > people will join us. LibExif is small library although quite complex. > I'd like to avoid blowing it up with complicated stacks or tables of > error handlers. Ack. Things are complicated enough as they are. If you want easy error handling, use an exception capable language :-) I think Mike just wants to design a little part of the API so he has a starting point for documenting it :) BTW, is anybody against me adding gtk-doc build stuff to libexif so that people with gtk-doc and willing to invest a few CPU cycles can build API docs? Gru=C3=9F, Uli |
From: Hans U. N. <gp...@n-...> - 2005-04-04 23:05:17
|
Lutz M=C3=BCller <lu...@us...> writes: > On Mon, 2005-04-04 at 23:24 +0200, Jan Patera wrote: >> To the complexity: I was originally thinking about upto 10 status >> codes, now my estimate is 10-15. > > Let's get down to business: Could you assemble this list? We can discuss > the prefix (EXIF_RESULT or EXIF_LOG_CODE, whatever) later. > > DEBUG: > - DATA > - MESSAGE > > WARNING: > - UNKNOWN_TAG > - BAD_FORMAT > > ERROR: > - NO_MEMORY > - DATA_AGAINST_SPECIFICATION > - TOO_FEW_DATA > - NO_EXIF_DATA > - BAD_PARAMETER - FILE_NOT_FOUND |
From: Lutz <lu...@us...> - 2005-04-05 05:44:24
|
DEBUG: - DATA - MESSAGE WARNING: - UNKNOWN_TAG - BAD_FORMAT ERROR: - NO_MEMORY - DATA_AGAINST_SPECIFICATION - TOO_FEW_DATA - NO_EXIF_DATA - BAD_PARAMETER - FILE_NOT_FOUND - FILE_NOT_READABLE --=20 Lutz M=FCller <lu...@us...> |
From: Lutz <lu...@us...> - 2004-05-17 18:18:52
|
On Sun, 2004-05-16 at 16:17, Joerg Hoh wrote: > I prefer a to replace at least some void-functions with (signed) int with a > standard return value of 1; if an error happens, they will return something > != 1 (normally < 1). (1) User clicks and empty information shows up. This is the case now. (2) User clicks and gets message: "An error occurred." This is your proposal. (3) User clicks and gets message: "Error xy occurred." Like EXIF_ERROR_CORRUPT_DATA, EXIF_ERROR_UNKNOWN_FORMAT (4) User clicks and gets message: "Error xy occurred at byte 4711 in EXIF data. The EXIF data in your image is broken. Please contact mailing list xyz." Implemented i.e. like void exif_data_load_data (..., ExifResult *result); where typedef struct { char *msg; ExifError err; } ExifResult; When doing error reporting, I'd like to do it in a userfriendly way, i.e. solution (4) or something better. Lutz |
From: Joerg H. <jo...@de...> - 2004-05-17 20:39:06
|
On Mon, May 17, 2004 at 08:18:56PM +0200, Lutz Müller wrote: > On Sun, 2004-05-16 at 16:17, Joerg Hoh wrote: > > I prefer a to replace at least some void-functions with (signed) int with a > > standard return value of 1; if an error happens, they will return something > > != 1 (normally < 1). > > (1) User clicks and empty information shows up. > This is the case now. > > (2) User clicks and gets message: "An error occurred." > This is your proposal. Nope. I don't talk about any kind of interaction with the user on application side. I talk about how errors in functions can be detected from a calling one. > (3) User clicks and gets message: "Error xy occurred." > Like EXIF_ERROR_CORRUPT_DATA, > EXIF_ERROR_UNKNOWN_FORMAT > > (4) User clicks and gets message: > "Error xy occurred at byte 4711 in EXIF data. > The EXIF data in your image is broken. > Please contact mailing list xyz." > Implemented i.e. like > void exif_data_load_data (..., ExifResult *result); > where > typedef struct { > char *msg; > ExifError err; > } ExifResult; I don't want to bloat the library. And yes, such a solution would be bloat. Why do I have to return a struct when no error occurred? How do I allocate a structure when a malloc(10) failed? Then we should have an error handling for the error handling... Returning a simple int in some functions is just enough. On high-level functions (which will be be called from application side) we can think about such a solution, but not on low-level. Joerg -- Fachbegriffe der Informatik (Nr 275): Luster-Format - Positiv gemeint: Eßfreudig Negativ gemeint: Das Äquivalent von zwei Öltanks Alexander Stielau |
From: Lutz <lu...@us...> - 2004-05-18 05:19:59
|
On Mon, 2004-05-17 at 22:39, Joerg Hoh wrote: > Nope. I don't talk about any kind of interaction with the user on > application side. I talk about how errors in functions can be detected from > a calling one. What does the calling function do with the information 'error occurred'? > Why do I have to return a struct when no error occurred? I thought about something like that: int main (...) { int i1, i2; ExifError e = {0, ""}; /* Calling a function without error checking */ exif_function (i1, i2, NULL); /* Calling a function with error checking */ exif_function (i1, i2, &e); if (e.id != 0) printf ("Error %i occurred: %s.\n", e.id, e.msg); } > On high-level > functions (which will be be called from application side) we can think > about such a solution, but not on low-level. As long as the low-level part of the library returns enough information to enable the user (or the one to whom the user complains) to understand the problem, I agree with you. Note that at the end of the chain of calling functions, there is always a user. Lutz |
From: Joerg H. <jo...@de...> - 2004-05-18 11:51:19
|
On Tue, May 18, 2004 at 07:20:07AM +0200, Lutz Müller wrote: > > > Why do I have to return a struct when no error occurred? > > I thought about something like that: > > int > main (...) { > int i1, i2; > ExifError e = {0, ""}; > > /* Calling a function without error checking */ > exif_function (i1, i2, NULL); > > /* Calling a function with error checking */ > exif_function (i1, i2, &e); > if (e.id != 0) > printf ("Error %i occurred: %s.\n", e.id, e.msg); > } That would be much work to do; I think at this moment it would be easier to work with return values. The errors I want to catch with this approach are relativly "simple" errors like error return values of libc-calls (malloc, realloc) and possibly invalid (NULL) arguments. Look at the points in functions where at this moment we just do a simple "return". Most times it's enough to distinct between "error occured" and "no error". There we don't need a highly-sophisticated concept how we want to do our error handling. We just need to detect them. On higher levels a solution like your proposal would be ok, but on the bottom level it's kind of overkill. Joerg -- Fachbegriffe der Informatik (Nr 68): WWW - World Wide Waiting |
From: Lutz <lu...@us...> - 2004-05-20 11:02:04
|
On Tue, 2004-05-18 at 13:51, Joerg Hoh wrote: > I think at this moment it would be easier to > work with return values. The errors I want to catch with this approach are > relativly "simple" errors like error return values of libc-calls (malloc, > realloc) and possibly invalid (NULL) arguments. Look at the points in > functions where at this moment we just do a simple "return". Most times > it's enough to distinct between "error occured" and "no error". I had an hour to play with libexif and came up with a proposal for debugging messages (exif-log.[c,h], try exif --debug non_existing_file.jpg). API didn't change. But now you have the possibility to watch what is going on internally using a callback-function. While thinking about how that could be used to satisfy your needs, I ran out of ideas regarding the following question: At which point does an invalid argument or a failed malloc become fatal? At libexif level? At the maker note level? I don't think a bug in the maker note code should let the loading of the ExifData fail. There seems to never be a 1/0-decision. Regards Lutz |