0

I'm using a library that, in order to construct some object that I use, expects a raw pointer to an object. I'm not sure what it will do with the pointer, to make my code as safe as possible, what should I pass to this function?

  1. Use a unique pointer - if they decide to delete the pointer, I will do a double-delete
  2. Keep track of a raw pointer - bad because I have to remember to write delete, but it could still be a double-delete
  3. Use auto duration and give them a pointer Give them a reference - their code will error if they call delete
  4. Use a shared pointer - same double-delete problem as unique pointer, but now my scope won't hurt their pointer

Based on my reading, option 3 seems like what I should do - they shouldn't be calling delete on the pointer, and this format enforces that. But what if I don't know whether they now or in the future will call delete on the reference I gave them? Use a shared pointer and say "not my fault about the double delete"?

#include <memory>
#include <iostream>

class ComplexObj {
  public:
    ComplexObj() : m_field(0) {}
    ComplexObj(int data) : m_field(data) {}

    void print() { std::cout << m_field << std::endl; }

  private:
    int m_field;
};

class BlackBox {
  public:
    BlackBox(ComplexObj* data) {
        m_field = *data;

        // Do other things I guess...

        delete data;
        std::cout << "Construction complete" << std::endl;
    }

    void print_data() { m_field.print(); }

  private:
    ComplexObj m_field;
};

int main(int argc, char* argv[]) {
    // Use a smart pointer
    std::unique_ptr<ComplexObj> my_ptr(new ComplexObj(1));
    BlackBox obj1 = BlackBox(my_ptr.get());
    obj1.print_data();
    my_ptr->print();  // Bad data, since BlackBox free'd
    // double delete when my_ptr goes out of scope

    // Manually manage the memory
    ComplexObj* manual = new ComplexObj(2);
    BlackBox obj2 = BlackBox(manual);
    obj2.print_data();
    manual->print();  // Bad data, since BlackBox free'd
    delete manual;    // Pair new and delete, but this is a double delete

    // Edit: use auto-duration and give them a pointer
    ComplexObj by_ref(3);
    BlackBox obj3 = BlackBox(&by_ref);  // they can't call delete on the pointer they have
    obj3.print_data();
    by_ref.print();

    // Use a shared pointer
    std::shared_ptr<ComplexObj> our_ptr(new ComplexObj(4));
    BlackBox obj4 = BlackBox(our_ptr.get());
    obj4.print_data();
    our_ptr->print();  // Bad data, they have free'd
    // double delete when our_ptr goes out of scope

    return 0;
}

Other questions I read related to this topic...

Jay
  • 93
  • 6
  • 9
    Why guess? Read the manual for the library to know if it's going to delete the pointer or not. – HolyBlackCat Jan 08 '21 at 18:13
  • 2
    _"Give them a reference - their code will error if they call delete"_ - Not sure what you mean here, if the function expects a pointer, you can't pass a reference. In `BlackBox obj3 = BlackBox(&by_ref);` `&ref` is not a reference, it's taking the _address of_ `ref`. – Lukas-T Jan 08 '21 at 18:15
  • 7
    Read their documentation. If they say they take ownership of the pointer, then you know you don't need to delete it. If they don't say anything, then generally assume you need to clean it up. If they're not documenting their stuff properly such that it leads to a memory leak or double-delete, then complain to them loudly. – TheUndeadFish Jan 08 '21 at 18:16

1 Answers1

2

You cannot solve this problem with the information you have. All choices produce garbage.

You have to read the documentation of the API you are using.

Doing any of your 4 answers without knowing if they take ownership of the pointer will result problems.

Life sometimes sucks.

If you have a corrupt or hostile API, the only halfway safe thing to do is to interact with it in a separate process, carefully flush all communication, and shut down the process.

If the API isn't corrupt or hostile, you should be able to know if it is taking ownership of the pointed to object. Calling an API without knowing this is a common mistake in novice C++ programmers. Don't do it. Yes, this sucks.

If this API is at all internal and you have any control, seek to make all "owning pointer" arguments be std::unique_ptr<>s. That makes it clear in the API that you intend to own the object and delete it later.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524