4

I recently came across an old C-function returning a pointer to an static array. I wrote a wrapper around that function and returned a std::unique_ptr that uses a no-delete to emphasize the type of pointer being returned - a "don't delete me" warning to the user. Here is a sample code

 extern "C" int *f(int i);

 struct noop
 {
      template <typename T>
      void operator() (T t) const noexcept
      {
      }
 };
 class MyClass
 {
     public:          
     std::unique_ptr<int, noop> F(int value) const 
      { 
            return std::unique_ptr<int, noop>(f(value));
      }
 };

Is there a cleaner way of doing this without defining a no-delete struct?

Catriel
  • 461
  • 3
  • 11
  • [`observer_ptr`](https://en.cppreference.com/w/cpp/experimental/observer_ptr) maybe. But why do you need to wrap a pointer to function in a smart pointer to begin with? – Praetorian Apr 18 '19 at 18:44
  • 7
    I think sticking with the raw pointer might be the best thing here. Raw pointers still have some use when you want your code to clearly show that you're just watching a memory location, and are not concerned with ownership. Your solution seems like unnecessary boilerplate. – alter_igel Apr 18 '19 at 18:49
  • 2
    You could also wrap the returned pointer in a [`span`](https://en.cppreference.com/w/cpp/container/span) that simply represents a view into contiguous memory, like a non-owning std::array or std::vector. Spans can be used with both a compile-time size and a dynamic size, depending on how much you know about the C function. This solution would also document your intent nicely. – alter_igel Apr 18 '19 at 18:53
  • 3
    Actually a better practice is to just never return/take a pointer that needs to be deleted. Return/take a `std::unique_ptr` or `std::shared_ptr` in these cases. So if you ever see a function that returns/takes a raw pointer, you know those are non-owning by convention. – Cruz Jean Apr 18 '19 at 19:00
  • If the array doesn't need to be freed, then there's no reason to use a smart pointer. Just use the raw pointer. Smart pointers are there to prevent memory leaks, which can't happen here. – Nikos C. Apr 18 '19 at 19:13
  • @NikosC. One reason can be not to allow the clients to have access to the raw pointer to delete it. – TonySalimi Apr 18 '19 at 20:07
  • @Hoodi Smart pointers give you full access to the raw pointer through the `get()` function. – Nikos C. Apr 18 '19 at 20:35
  • @NikosC.I mean, in some cases the developer of a library does not want the user to have access to raw pointers, instead prefers to warp them on something. – TonySalimi Apr 18 '19 at 20:38
  • @alterigel, I was dealing with legacy code, and spent twenty minutes going through various levels of calls and looking for examples where the pointer was called, where was it returned trying to figure out if I was supposed to delete the memory or not. In my particular case, specifying the type of pointer being returned made a lot of sense. – Catriel Apr 22 '19 at 17:49
  • @NikosC. I'm with Hoodi. Yes, you can always call .get(), but, in this case, you are being malicious. If the pointer simply states observer_ptr, the caller gets a good indication that there is no need to delete after using it's value. – Catriel Apr 22 '19 at 17:52

2 Answers2

6

[..] an old C-function returning a pointer to an static array. I wrote a wrapper around that function and returned a std::unique_ptr

Don't. Returning std::unique_ptr says to the caller:

  • you own this memory, clean up after yourself
  • you own this memory, nobody else will interfere (thread safety)
  • you own this memory. Thus you get a new object every time you call (the wrapper function).

None of this is true for a pointer to a static array!

Stick with the raw pointer or use some wrapper class to allow reference-like access. std::reference_wrapper to also provide semantics like the raw pointer when copying for example.

Daniel Jour
  • 15,896
  • 2
  • 36
  • 63
  • 2
    @FrançoisAndrieux Indeed, but returning a unique pointer from a function IMO should have that constraint; or at least have a thread safe implementation in the pointed to type. If one thread owns an object and other threads are using that, then I'd go with a `shared_ptr` / `weak_ptr` combination. – Daniel Jour Apr 18 '19 at 19:12
  • 1
    Just consider what happens when you destruct the unique_ptr. The pointed to object will then also be destructed, regardless of what other threads are currently doing with it. Unless you put locking into the destructor ofc – Daniel Jour Apr 18 '19 at 19:14
  • 2
    You're right that, in the context of a function's return type, you have to assume there exists no other reference to the pointed object. Otherwise, lifetime is not entirely in your hands so ownership isn't unique. – François Andrieux Apr 18 '19 at 19:20
  • Suppose that wrapping the raw pointer is inevitable by any reason, let's say user-friendliness, delete avoidance, whatever. How about wrapping the pointer in a simple object that overloads operator -> ? – TonySalimi Apr 18 '19 at 20:31
  • This construct is great, but does not address in a clean manner the case where the pointer being returned is a null pointer. I have to think more about this. – Catriel Apr 22 '19 at 14:37
  • 1
    @Catriel https://en.cppreference.com/w/cpp/experimental/observer_ptr may hit Standard some day. Although, as explained here: https://stackoverflow.com/a/24158171/5945883 it is just alias for `T*` – R2RT Apr 22 '19 at 15:08
  • @R2RT, yes, the observer_ptr is exactly what I need. I followed your link, and think that I might follow template observer_ptr = T *; as an answer to my current situation. – Catriel Apr 22 '19 at 18:21
  • Hm ... there are some sudden downvoters? Can you explain why? Anything new on this which should be reflected in my answer? – Daniel Jour Mar 14 '22 at 17:40
0

The C++ coding guidelines suggest using aliases or "dummy" wrappers to indicate ownership, or non-nullness:

For their implementation, have a look at:

https://github.com/Microsoft/GSL/blob/master/include/gsl/pointers

You'll see the example of owner<> and of non_null<>.

You could similart definition. Clearly, a non-owning pointer must not be deleted; if you just want a reminder, then

template <class T, class = std::enable_if_t<std::is_pointer<T>::value>>
using non_owner = T;

and if you want enforcement, copy and adapt the code for non_null<> from the link.

einpoklum
  • 118,144
  • 57
  • 340
  • 684