Hi Christian, On 12/7/2010 9:18 PM, Christian Henning wrote:
2. is it wise to drag the stuff from e.g. the IJG JPEG library's public headers in to client scope? I ask because I've had clashes with the "boolean" macro before and this would outright prevent me from using the library. The implication of addressing this would probably mean that the GIL would no longer be header-only. That's fine by me, but perhaps it's a design goal of the GIL for some reason?
Would a namespace do the trick?
In what way?
(FWIW, after checking I now see "boolean" is a typedef rather than a macro, but there was a clash nonetheless).
I meant something like:
namespace libjpeg {
extern "C" { #include<jpeglib.h> }
} // namespace libjpeg
Would this work if libjpeg was compiled with a C++ compiler? (though perhaps that won't be supported, in the end).
- What is your evaluation of the implementation?
Looking again at the JPEG code here. As far as I can tell setjmp/longjmp have been used with appropriate care, but I worry what will happen if a method on a Device throws an exception, for example. I've had both Visual C++ 2008 and Apple's g++ 4.0.1 crash on me in this kind of situation.
Do you mean when an std::fstream throws an exception or when FILE issues an error?
I was thinking of when a stream (or streambuf) of whatever kind throws an exception.
I think the user should catch the exception. io_new is also throwing exceptions when needed.
They might not have a chance when an exception is thrown from a function that is called from C (one of the callbacks installed in jpeg_decompress_struct, for example). All (portable) bets are off in this situation, as far as I know. This is where the boost::current_exception() suggestion comes from. You can catch and store the exception inside the callback (as best as possible) and then rethrow it when you're safely back in C++ land.
- What is your evaluation of the documentation?
Couldn't find a lot. Either my eyes are poor, or the documentation is :)
Did you see the .qbk file in the /doc folder?
Gah, no. My eyes, then! I must have got lost in the deep folder hierarchy.
;-) I should have build the documentation before the review. It's my fault.
I forgot to say in my previous reply, but this now changes my vote as to whether the library should be included to a Yes. Kind regards, Edd