On Saturday, July 25, 2015 at 1:16:11 PM UTC-4, jean-pierre.muench wrote: > > Hey everyone, > > I've gone through all the stuff. (not the special dll import stuff yet) > > I could fix a few things without much trouble however there were three > issues that were outstanding. > > > 1. The destructor of AlgorithmParametersBase. It can throw under > specific circumstances. However destructors are assumed not to throw and I > think it's considered bad style if they do (there's no one to catch > usually). Here's the code and how I disabled the warning for now (not in > the proposed patch)... > > OK, at 10,000 feet, there are a few things going on here:
(1) What does C++ provide or recommend (2) What the library guarantees (3) What users expect Related to (1) we also have: (4) What version of C++ are we working with (or against)? So, before we move to implementation changes, I think we should understand the intersection of (1) - (3). For (1), we can start at https://stackoverflow.com/questions/130117/throwing-exceptions-out-of-a-destructor. But we also need to know what has changed with C++11 and C++14. In the case of (2) and (3), AlgorithmParameters has a contractual obligation to throw (modulo some conditions, like the user specifically asked throwIfNotUsed = false). So its likely going to throw in some circumstances, even if its unpalatable for others. ***** For AlgorithmParameters, maybe we can change some internals so that the implementation is tear off (similar to a PIMPL): class AlgorithmParameters { AlgorithmParameters(bool throwIfUnused); ... private: AlgorithmParametersImpl* m_impl; } AlgorithmParameters::AlgorithmParameters(bool throwIfUnused) : m_impl(AlgorithmParametersHelper(throwIfUnused) { } AlgorithmParametersImpl* AlgorithmParametersHelper(bool throwIfUnused) { if(throwIfUnused) return new AlgorithmParametersThrowIfUnused; else return new AlgorithmParametersDoNotThrowIfUnused; } ***** So here's what I was thinking for the immediate future: (A) we need to study it some more before we move forward. ***** Looking long term, I despise this pattern: try { ... } catch(...) { // Swallow everything } As a matter of C++ policy, I'll begrudgingly agree to catching and swallowing a "CryptoPP::Exception&" in dtors. I'll agree because its working and playing well with others, so it speaks to (3) and addresses (1) above. However, I don't think we should catch and swallow a "std::exception&" or "std::runtime_error&" or "...". The library is layered on top of the runtime, and there usually no way the library can claim it knows how to handle a C++ runtime problem. If the library knew there was a problem, then it should have avoid the state in the first place... -- -- You received this message because you are subscribed to the "Crypto++ Users" Google Group. To unsubscribe, send an email to [email protected]. More information about Crypto++ and this group is available at http://www.cryptopp.com. --- You received this message because you are subscribed to the Google Groups "Crypto++ Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
