-1

I want to use the vector to collect some generated ints, the number of which I don't know until runtime. However, the interface I have to implement requires returning a native pointer which is supposed to be freed by the caller. The vector is going to be quite large, so, to avoid copying, I do the following:

std::vector<int>* const Collector = new std::vector<int>;
//Several Collector.push_back();s
int* ReturnPointer = Collector->data();
free(Collector);
return ReturnPointer;

The code above is meant to avoid the vector's deconstruction which will free the data pointer that I have to return - and the vector controller itself is freed.

My question is: is this trick safe? Or is there a better solution?

IMPORTANT: The full architecture is out of my control. I'm not allowed to return something other than a native pointer or require the callers to change their calling code. That is, I can't return that vector.

I know that it's bad to free a newed pointer. Then what about doing:

std::vector<int>* const Collector = (std::vector<int>*)malloc(sizeof(std::vector<int>*));
*Collector = std::vector<int>;
//Several Collector.push_back();s
int* ReturnPointer = Collector->data();
free(Collector);
return ReturnPointer;

This time I freed the malloced pointer, is this OK?

cigien
  • 57,834
  • 11
  • 73
  • 112
埃博拉酱
  • 93
  • 6
  • 4
    `free(Collector);` is undefined behaviour. You *must* `delete` what you `new`ed – Caleth Jun 17 '22 at 11:03
  • This will not work. Attempting to`free` what's `delete`d always ends in tears. But there's good news: correct usage of move semantics in modern C++ completely deals with the problem you're trying to solve in a completepy proper, type-safe way. See your C++ textbook's chapter on move semantics for more information. Even though, for example, NVRO is optional most modern compilers will employ it, when it's teed up correctly, completely eliminating a vector copy when returning it from a function. – Sam Varshavchik Jun 17 '22 at 11:03
  • 1
    @SamVarshavchik It seems you overlooked the question, move semantics will be of no help here. OP wants not to return a vector but a pointer to it's internal data instead. As written (and forgetting the illegal `free()`), either the vector remains alive which leads to memory leak, or the vector is destroyed (if we `delete` it) and the returned pointer is now dangling. – Fareanor Jun 17 '22 at 11:24
  • 2
    Think of a `std::vector` as a *smart pointer*, which encapsulates a dynamic (resizable) array, with container semantics (`begin()`, `end()`, iterators, et cetera). Just as it makes no sense to `new std::unique_ptr`, likewise it makes no sense to `new std::vector`. – Eljay Jun 17 '22 at 11:46
  • @Eljay A vector is very different from a smart pointer in that it doesn't have a method to release its pointer. – 埃博拉酱 Jun 17 '22 at 11:56
  • Correct, the vector does not release the pointer. But it can transfer the pointer using `std::move`, such as in `std::vector v1{Foo{}, Foo{}, Foo{}, Foo{}}; std::vector v2 = std::move(v1);` – Eljay Jun 17 '22 at 12:26
  • 1
    @埃博拉酱 `shared_ptr` doesn't have release either. – apple apple Jun 17 '22 at 12:39
  • See also: https://stackoverflow.com/questions/58415262/intentionally-leak-the-memory-of-a-stdvector – Cody Gray - on strike Jun 23 '22 at 22:25

1 Answers1

5

You can, but not with std::vector<int>. You need an allocator that is compatible with the deallocation, and that will not deallocate the final array.

If you new, new[] or malloc you must respectively delete, delete[] or free, mixing those is undefined behaviour. You must check how the interface deallocates the return value.

You also don't need to new the local vector.

template <typename T>
class releasing_allocator {
    bool * released;
public:
    using value_type = T;

    releasing_allocator(bool * released) : released(released) {}

    T * allocate(std::size_t n) { return new T[n]; }
    void deallocate(T * p, std::size_t n) { if (!*released) delete[] p; }    
};

bool done = false;
std::vector<int, releasing_allocator<int>> Collector{ releasing_allocator { &done } };
Collector.push_back(42); // etc
done = true;
return Collector.data();

Because of these issues, it is typical to require the caller to allocate whatever they will deallocate.

Caleth
  • 52,200
  • 2
  • 44
  • 75
  • Your answer is elegant, but what about my updated one? This time I really malloced the memory for the vector and freed it respectively. – 埃博拉酱 Jun 17 '22 at 11:48
  • If the container is not a vector but a valarray which does not accept a custom deallocator, my solution seems to be the last choice? – 埃博拉酱 Jun 17 '22 at 11:50
  • @埃博拉酱 That's even more undefined behaviour. You *don't have* a vector behind that `std::vector*`, and you *don't need* to dynamically allocate your vector. – Caleth Jun 17 '22 at 11:50
  • @埃博拉酱 no, you simply can't use `std::valarray` like that. – Caleth Jun 17 '22 at 11:52
  • And the distinction of `new[]` vs `malloc` is for the `int`s, not the vector. The vector ceases to be, calls `releasing_allocator::deallocate`, which doesn't enter the true branch of the `if`, and thus the `int`s are still around for the caller to deallocate. – Caleth Jun 17 '22 at 11:56
  • Importantly, any reallocations of the vector as you fill it will deallocate when the vector grows – Caleth Jun 17 '22 at 12:00