Closed melax closed 8 years ago
Agreed - this is a bit of an anti-pattern since it appears this will be compilable from source and not as a dynamically linked blob (code in question: https://github.com/IntelRealSense/realsense_sdk/blob/a37b6628c416b45309a91ca96a8d380700d98e9b/sdk/include/rs/utils/smart_ptr.h)
I third the use of shared_ptr, which would also fix a number of defects in this code.
Presently, it is vulnerable to accidental double-ownership issues, as it has both a non-explicit constructor and an assignment operator taking a bald pointer, which both assume ownership semantics by allocating a new reference count. The STL implementation only ever assumes ownership on explicit construction from bald pointer, making it much harder to accidentally double-free an object.
We are still not sure how to finalize this API, as we would indeed like to use std. However, this code is going to be used by MWs libraries, which are compiled with intel compiler and provided as binaries. We are looking for a way to assert compatibility so we can use std, and if you have any advice here we'd be happy to get it. We'll contact you internally with our requirements and restrictions, so you will have the full picture.
Thanks for you comments!, the smart ptr was removed, instead, for ABI safety, we added a pure virtual reference counting interface + basic implementation.
if smart pointers are needed, use std::shared_ptr rather than recreating a custom version.
In general, stick to the standard library.