Re: [boost] [fiber] ready for next review

On 12/4/2015 11:48 AM, Agustín K-ballo Bergé wrote:

On 12/4/2015 4:47 AM, Oliver Kowalke wrote:

...

2015-12-03 22:21 GMT+01:00 Agustín K-ballo Bergé :

...

Yes, in the documentation.

documentation might need some updates - my announcement was primarly focused to the source code

Fair enough, I was misguided by the "ready for next review" subject as well as the announcement that requests from the review have been addressed. This is obviously not the case, but we can still make a lot of progress based on source code adjustments only.

The destructor for `shared_state` contains the following code: if ( ready_) { reinterpret_cast< R const* >( storage_)->~R(); } As far as I can see, `ready_` will be set both when the shared state holds a value or an exception. This would mean a shared state holding an exception would run arbitrary code on random garbage during destruction (sidenote: how did this manage to eluded unit tests??). The choice of a cast to `const` pointer for destruction is peculiar. Not wrong, just odd, since qualifiers stop being in effect when the destructor starts. The choice of `reinterpret_cast` is questionable. It would be safer to replace it with the two `static_cast`s this use case is supposed to synthesize. It'd probable be good too to replace the "redundant" artificial array of one storage this `reinterpret_cast` is relying on. ...or maybe just use `optional<R>`? Furthermore, the member function `reset` will simply set the `ready_` flag to `false`, leaking the result value if there is one. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com