Improve exception safety with smart pointers

[elfring]

💭 Would you like to wrap any pointer data members with the class template “std::unique_ptr”?

[braindigitalis]

no thanks, this will add extra complication to the library, and the library itself tracks the ownership of the pointers (and owns all pointers).
In fact we barely use any manual allocation via new at all, except in caches and a couple of internal values.

[elfring]

💭 Would you become interested to apply C++ Core Guidelines for any more software components?

[elfring]

In fact we barely use any manual allocation via new at all, …

💭 How does such a feedback fit to improvable source code places?

Example:

DPP/src/dpp/discordclient.cpp

Line 103 in d4b74da

zlib = new zlibcontext();

[braindigitalis]

In fact we barely use any manual allocation via new at all, …

💭 How does such a feedback fit to improvable source code places?

Example:

DPP/src/dpp/discordclient.cpp

Line 103 in d4b74da

zlib = new zlibcontext();

there really is no need for this to be a smart pointer, it is just added complexity. Its lifetime is managed by its owning class.

[elfring]

💭 Are smart pointers provided for the avoidance of undesirable software behaviour?

👀 Where do you see owning objects for the pointers “etf” and “zlib”?

[braindigitalis]

the one that creates them, discordclient :) This is the only place they are instantiated.

[elfring]

This is the only place they are instantiated.

I know.

I interpret the linked source code in the way that the two pointer member variables “etf” and “zlib” are not initialised with “nullptr” so far.
Thus such a pointer combination is generally problematic in case an exception like “std::bad_alloc” would be thrown.

[elfring]

👀 Would you like to take another look at the advice “Item 10: Prevent resource leaks in constructors.”?

[braindigitalis]

changing everything to smart pointers is a potentially lazy option, the correct fix is shown here without needing to add complexity to the library.

3981360

We simply need to ensure that nullptr is set for those two values, and rethrow the std::bad_alloc after any cleanup. This is a much simpler change than refactoring an entire stable library (where a bad_alloc on construction of a discordclient has never happened in production in a real world system to our knowledge) to use unique_ptr.