0

How can a class know, that it should not free its allocated memory in the destructor. Because the class is a pointee to a pointer?

Some example code (untested) that does not work. The problem is at the constructor of MyOtherClass.

class MyClass {
    ~MyClass() { free(...); } // free some vars, does not matter what.
};
class MyOtherClass {
    MyClass *m_ptr;
    MyOtherClass(MyClass pointee) {
        m_ptr = &pointee;
        // when this constructor is finished, the destructor of parameter "pointee" will be called.
        // and therefore the attribute "m_ptr" points to nothing. 
    }
};

Could it be possible that MyClass uses something like this?

class MyClass {
    bool i_am_a_pointee = false; // but how and where to assign this?
    ~MyClass() { 
        if (!i_am_a_pointee) { free(...); }
    }
};

The goal is to create a container like class for multiple different classes. But with pointers.

Code:

class MyClassA {
    MyClassA() {}
    // here the pointer is cleared because that is required for my class. 
    // So when the var goes out of scope like in the constructor of "MyContainer(MyClassB)" all the vars will be deleted and therefore the pointer technically points to nothing.
    ~MyClassA() { free(...); } 
};
class MyClassB {
    MyClassB() {}
    // here the pointer is cleared because that is required for my class. 
    // So when the var goes out of scope like in the constructor of "MyContainer(MyClassB)" all the vars will be deleted and therefore the pointer technically points to nothing.
    ~MyClassB() { free(...); }
};
class MyContainer {
    MyClassA *m_class_a;
    MyClassB *m_class_b;
    MyContainer(MyClassA x) {
        m_class_a = &x // here the problem.
    }
    operator MyClassA() {
        return *m_class_a;
    }
    MyContainer(MyClassB x) {
        m_class_b = &x // here the problem.
    }
    operator MyClassB() {
        return *m_class_b;
    }
};
int main() {
    MyContainer container;
    MyClassA a;
    MyClassB b;
    container = a; // here the pointer is empty because of the problem.
    MyClassA a1 = container;
    container = b;
    MyClassB = container;
}
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • 4
    It's none of the class's business to know that, so it doesn't. – Sam Varshavchik Jul 12 '22 at 23:58
  • But how can you manage that it does not free its memory then? – user3134709 Jul 13 '22 at 00:00
  • 2
    The constructor builds the instance in whatever storage was provided for the instance and has no part in the allocation of the storage. Likewise the destructor has no part in releasing the storage and has no idea where it came from. When you call `new`, it allocates the memory and then calls the constructor. When you call `delete`, it calls the destructor and then frees the storage. With this separation of responsibilities you can automatically allocate, dynamically allocate, and place the instance in existing storage all with the same constructor and destructor. – user4581301 Jul 13 '22 at 00:01
  • @user4581301 Then what is the purpose of the destructor? – user3134709 Jul 13 '22 at 00:05
  • So should i declare `m_ptr` with new statements, and then it could work? Or is that not what you mean? – user3134709 Jul 13 '22 at 00:06
  • 2
    The destructor performs any necessary clean-up and then frees any resources owned by the instance. But it does not free the instance itself. – user4581301 Jul 13 '22 at 00:07
  • Btw, the destructor also gets called when a variable passes out of scope like in this example. – user3134709 Jul 13 '22 at 00:07
  • 1
    `MyOtherClass(MyClass pointee) {m_ptr = &pointee;}` is undefined behavior waiting to happen. `m_ptr` is initialized to point to a local variable, and becomes dangling as soon as `MyOtherClass` constructor returns. Any attempt to use `m_ptr` would exhibit undefined behavior. This arrangement doesn't make any sense. – Igor Tandetnik Jul 13 '22 at 00:09
  • No i know. But i am allocating memory inside the `MyClass` and freeing it in the destructor. Without pointers everything works fine, but with pointers it calls the destructor of the pointee so the pointer is empty. This example in a simple main without a function would work because then the vars go out of scope at the same time. – user3134709 Jul 13 '22 at 00:09
  • How should it be arranged then? – user3134709 Jul 13 '22 at 00:09
  • 2
    `m_ptr` should be a non-owning pointer because the class does not know where the object it's pointing at came from. If properly documented, it could be an owning pointer and take responsibility for the destruction of the object, but this is typically described by a `std::unique_ptr` in modern C++. In the case of `MyOtherClass(MyClass pointee)`, `pointee` was passed by value and this is a locally-scoped variable. It will expire at the end of the constructor. Storing a pointer to it is likely a fatal mistake. – user4581301 Jul 13 '22 at 00:10
  • 2
    That rather depends on what you are trying to achieve. What *should* `m_ptr` point to? Show a [mcve] – Igor Tandetnik Jul 13 '22 at 00:10
  • Okay, agreed. In what (user friendly) way should i define the `m_ptr` attribute then? – user3134709 Jul 13 '22 at 00:11
  • @IgorTandetnik Well to an instance of `MyClass` – user3134709 Jul 13 '22 at 00:12
  • 1
    Where and how is this instance of `MyClass` created? Who owns it, who controls its lifetime? Again, show a [mcve] – Igor Tandetnik Jul 13 '22 at 00:12
  • 1
    Handy reading: [What is ownership of resources or pointers?](https://stackoverflow.com/questions/49024982/what-is-ownership-of-resources-or-pointers) The owners of all resources need to be established fairly early in the design of a program. – user4581301 Jul 13 '22 at 00:15
  • I have added a clear goal code. At least i hope it is clear. – user3134709 Jul 13 '22 at 00:17
  • `a` and `b` are automatic variables. You could say they own themselves and will automatically (hence the name) be destroyed and freed when they go out of scope at the end of the block in which they were defined. – user4581301 Jul 13 '22 at 00:18
  • `MyContainer` constructors should take their argument by reference, not by value. As in `MyContainer(MyClassA& x)` (note the ampersand) – Igor Tandetnik Jul 13 '22 at 00:19
  • Exactly, that is the problem. How can i manage that they do not delete themselves if they are pointee's? – user3134709 Jul 13 '22 at 00:20
  • 1
    There's nothing you can put in a destructor to magically extend the lifetime of a local variable past the end of the block in which it's declared, if that's what you are asking. – Igor Tandetnik Jul 13 '22 at 00:21
  • You need to pass by reference: `MyContainer(MyClassB & x)` so that the constructor will be operating on the original object and not a copy. You will still not need to manually destroy it because it is owned by someone else. – user4581301 Jul 13 '22 at 00:22
  • So when you declare a parameter by reference it does not create a copy? But then this is like 100* times faster, for most funcs? – user3134709 Jul 13 '22 at 00:24
  • If you `MyContainer(MyClassB * x) { m_class_b = x; }` and pass in a pointer to a `MyClassB` you have to establish who the owner of the `MyClassB` is. and they will delete it. If `MyContainer` is intended to take ownership, the `~MyContainer` must `delete m_class_b;`, but you have to be absolutely certain no one else thinks they have ownership. That means you cannot `MyClassB b; MyContainer container(&b);` because `b` owns itself. – user4581301 Jul 13 '22 at 00:27
  • Allright thanks for all the help! Passing by reference does indeed fix the problems. For now... lol – user3134709 Jul 13 '22 at 00:29
  • 1
    Passing by reference is usually faster, but there are cases where it isn't. Time it to A) make sure it's faster and B) it's faster enough to be worth it. But first and foremost make sure it describes the behaviour you need. Fast code that doesn't work properly is useless. – user4581301 Jul 13 '22 at 00:30
  • 2
    In general, an object (instance of a class) has no reason to check if it is a "pointee". It is responsible for releasing any resources (memory, mutexes, etc) that *it* manages. This is achieved by constructors allocating the resource(s), destructor releasing those resources, and any other member functions not screwing things up (e.g. don't release the resource unless they reallocate). None of that needs an object to know it is a pointee. If an object `X` *is* a pointee, then the code (other objects) holding the pointer manages `X`s lifetime, and `X` manages its own resources. – Peter Jul 13 '22 at 00:33
  • 1
    Another bit of important reading: [What is meant by Resource Acquisition is Initialization (RAII)?](https://stackoverflow.com/questions/2321511) The general gist is when you have a resource, manage the resource as close to the resource as possible. If you have a resource that doesn't know how to clean up after itself, make a small *wrapper* class that looks after it and does nothing else. It acquires the resource, releases it, makes sure it is copied and moved correctly. That way other classes don't need to worry about it. They can use the resource through the wrapper and focus on their job. – user4581301 Jul 13 '22 at 00:38
  • In modern **C++**, one should avoid raw memory managment which is somewhat error prone and instead use `unique_ptr` or `shared_ptr` for managing dynamically allocated memory. In C++, **you are** essentially responsible as the programmer to properly manage memory. Essentially, each time you allocate memory with `new`, you should free it with `delete`. If you use `new[]` then you need to call `delete[]`. **C** `free()` function is normally not used from C++ code. – Phil1970 Jul 13 '22 at 02:44

2 Answers2

2
MyOtherClass(MyClass pointee) {
    m_ptr = &pointee;
}

MyContainer(MyClassA x) {
    m_class_a = &x // here the problem.
}

Here you are taking pointee / x by value, that means the compiler will make a copy. You need to define a meaningful copy constructor for that to work with pointers.

Then you take the address of that argument, which is a local stack variable. At the end of the function that variable goes away and your pointer is dangling. No matter how correct you implement MyClassA this can never work.

You need a pointer or reference there so you can get the address of the object from the caller instead of the local variable. I would suggest smart pointers.


Never store raw pointers. There are too many ways to mess that up. When a class is the sole owner of a pointer then use unique_ptr. When you need to store the pointer in multiple places then use shared_ptr. If unsure, like when returning a pointer the user might or might not store, default to shared_ptr.

Apart from avoiding errors this also simplifies your code. Often you don't need a destructor because unique_ptr/shared_ptr handle the cleanup for you.

Goswin von Brederlow
  • 11,875
  • 2
  • 24
  • 42
1

How can a class know, that it should not free its allocated memory in the destructor. Because the class is a pointee to a pointer?

The short answer is that if you want to hold a raw pointer that may or may not be an owning-pointer, then you'll need to track that information somehow. As a crude approach, you could have a boolean member-variable named delete_pointer_in_destructor and set it to true iff the pointer should be deleted in the destructor, or false if it should not. If the object was allocated somewhere else in your program, then the calling code might have to pass in a value of this boolean as an argument (along with the pointer) to tell your code what it needs to do, since there would be no way for your code to know, just from the raw-pointer alone, which setting would be appropriate.

A much better and less error-prone approach, OTOH, would be to avoid raw owning-pointers altogether -- either store objects by-value only (in which case you don't ever need to call new or delete at all), or (if you can't do that, e.g. because you need polymorphic behavior) then at least use smart pointers like std::shared_ptr or std::unique_ptr as they will handle all of the decisions about when (or if) to call delete for you, and spare you the risk of getting those decisions wrong.

MyOtherClass(MyClass pointee) {
    m_ptr = &pointee;
    // when this constructor is finished, the destructor of parameter "pointee" will be called.
    // and therefore the attribute "m_ptr" points to nothing. 
}

You should try to avoid ever keeping a pointer to an object past the time the object has been destroyed; holding a dangling pointer is useless and asking for trouble, since any attempt to actually use the dangling pointer will invoke undefined behavior. So the fix for the above code would be to set m_ptr to NULL at the end of the constructor (or better yet, never set it to &pointee in the first place)

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234