Re: [boost] Half-baked GIL IO extension review

Subject: Re: [boost] Half-baked GIL IO extension review
From: Christian Henning (chhenning_at_[hidden])
Date: 2010-12-07 16:18:34

Hi Edd,

>>> 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

I had the impression your compiler is complaining about redefinition
of "boolean".

>
>>> - 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.

>
>>> In my code, I attempt to store an exception by doing something akin to
>>> boost::current_exception(), then longjmp out to a safe point before
>>> re-throwing it.
>>
>> Mhmm, I have to admit I'm unaware of boost::current_exception. Can you
>> point where you use it in your code?
>
> I don't actually use boost::current_exception in my code (it doesn't depend
> on boost at all, currently), but I do something very similar. Here, for
> instance:
> http://bitbucket.org/edd/jpegxx/src/ea2492a1a4a6/src/read.cpp#cl-166
>
> The exact type and behaviour of the ex_store_ object there depends on the
> preprocessor definitions that are present when the library is built. See:
>
> http://bitbucket.org/edd/jpegxx/src/ea2492a1a4a6/src/best_exception_store.hpp
>
> And for the various exception_store implementations:
>
> http://bitbucket.org/edd/imagexx/src/5c44e8ad57e7/include/imagexx/exceptions/

I'll have a look. Thanks for pointing out!

>
>>> - 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.

Regards,
Christian