On 6/28/19 11:08 AM, Andrzej Krzemienski via Boost wrote:
Hi Everyone, This is not a review of out_ptr, but I wanted to share my thoughts about the motivation for this library. I do not recall any time from my programming experience where I was in need of passing a smart pointer to a T** interface, so maybe this is why I cannot appreciate the value of out_ptr.
On the other hand, if I need to use a library with a C API, or other non-C++ API (this happens when someone wants to naively port a Java library to C++ and keep the Java interface) the first thing I do is to wrap the API functions into a RAII class. It takes some time up front, but then whenever it is used, I do not have to deal with the problems of the old API. `T**` is only one of the problems.
The motivation focuses on only easily passing smart pointers to `init(T**)` functions, but it does not address the problem of adapting the error reporting of `init(T**)`to the error reporting of my program. I would never want to only solve the former problem and omit the latter. Also, I would never want to represent a handle to a resource with a smart pointer, as I do not want to voluntarily inject the null-pointer state that I now have to take care of everywhere. Instead I would prefer a dedicated type that is not default-constructible, where one really needs to try hard to get the special no-resource state (e.g. by moving from and not destroying the object).
What I am missing from the motivation of out_ptr is the comparison with a full RAII wrapper solution. You can argue that the scope of out_ptr is just different: provide interpoperation between `init(T**)`functions and smart pointers. But I would argue that this latter goal is wrong. In fact, I am concerned that we might be encouraging a bad programming style: "need to work with init(T**) APIs? use shared_ptr." instead of promoting RAII wrappers that directly wrap the resource.
Does anyone else feel the same? Or am I missing something about the problem domain? Anyway, I think that this problem should be covered in the documentation.
In my experience, a full hand-rolled RAII wrapper is an overkill in many, if not most cases. Most C-style APIs follow the new/delete pattern, where you have a function that allocates and initializes a structure, returning a pointer to it, and another one to free it, along with any associated resources. Using a smart pointer to store a pointer to that structure is perfectly valid, and the easiest and most natural thing to do. Granted, Boost.OutPtr is aimed at only a subset of these APIs, which return the allocated pointer via an out parameter, not a function return value. There are other examples beyond new/delete pattern, like COM, for instance, which has the convention to return pointers to interfaces in the out parameters (because the return type is reserved for HRESULT). I'm sure there are other APIs that follow a similar convention. The important thing to note here is that the returned pointer represents exactly that - a pointer to something. And using a smart pointer to manage that resource is perfectly adequate. Boost.OutPtr tries to simplify the transition from the raw C pointer form to the smart pointer form and make it safer. Also, I think adapting error reporting mechanism is an entirely different task. You may implement it as part of the same component (e.g. a wrapper around the library), but it is still a separate thing to do, unrelated to RAII and pointer adaptation.