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

Subject: Re: [boost] Half-baked GIL IO extension review
From: Edd Dawson (lists_at_[hidden])
Date: 2010-12-07 15:29:59

Hi Christian,

On 12/7/2010 3:49 PM, Christian Henning wrote:

> You can use string, wstring, char*, and filesystem::basic_path ( when
> compiling with BOOST_GIL_IO_ADD_FS_PATH_SUPPORT ) to define a
> filename. But the wide character strings are then converted back
> std::string. Though, I have to admit that reading a unicode filename
> isn't supported yet. I'll make this a top priority.
>
> Basically I have to add some overloads that use wfopen. Or is there more to do?

For me, that's probably sufficient.

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

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

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

>> - 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 think most of your concerns will be taken care once the review has
> finished. I'll let you know.

Thanks!

Kind regards,

Edd