0

I think this is probably a really simple question, but I am as much a C++ developer as the guys at the walmart meat counter are butchers.

Say I have:

class Parent{
    protected:
        ~Parent(){};
};

class ChildA : public Parent{

};

struct Container{
    Parent *child;

    //Tried this, causes: munmap_chunk(): invalid pointer
    ~Container(){
        delete &child;
    }
};

Container MakeStruct(){
    ChildA child = *new ChildA();
    return Container { .child = &child };
}

int main()
{
    Container cont = MakeStruct();

    //Tried this, causes: "Parent::~Parent()" is inaccessible
    delete cont.child;
}

As you can see, I am using new to create a ChildA because I need it to outlive the MakeStruct function. So I know this means that child (in MakeStruct) will be placed on the heap, and that I am responsible for deleting it. But I can't seem to delete it. I can't change the type of child in Container because I need it to accept both a ChildA and a ChildB. It sort of makes sense, considering the destructor of Parent is protected. I have no control over Parent or either Child. They are part of an external library.

I case it helps, the actual code I'm working with is a library called ArduinoJson.

I am trying to return either a DynamicJsonDocument or a StaticJsonDocument<T> from a function, wrapped in a struct taking a JsonDocument:

Here is the struct that contains the JsonDocument:

struct rpc_handler_result_t {
    esp_err_t result;
    JsonDocument *response;
};

which is returned from:

{
    const int len = JSON_OBJECT_SIZE(1);
    StaticJsonDocument<len> reply = *new StaticJsonDocument<len>;

    reply["MaxOutput"] = Configuration::GetMaxOutputAmps();

    rpc_handler_result_t res = {
        .result = ESP_OK,
        .response = reply
    };

    return res;
}
Matthew Goulart
  • 2,873
  • 4
  • 28
  • 63

1 Answers1

1

When you eventually call delete, you must pass it precisely the value you got from new. So you must store the value that new returns somewhere. But look at your call to new -- it dereferences that value and never stores it anywhere. So how can you call delete?!

 Container MakeStruct(){
    ChildA child = *new ChildA(); // The value returned by new is lost here
    return Container { .child = &child }; // child is a local object here
}

This is all wrong. You create a new object by calling new. But you do not store the value new returned anywhere. Now, child is a temporary object whose value was copy-constructed from the value of the object you allocated with new and leaked.

Then, you save the address of the temporary child object you created on the stack. But that object won't exist after you return.

What you wanted to do was save the value that new returned. But you got rid of it immediately by dereferencing it and never saving it.

So:

  1. You must store the value that new returns so you can delete it later.
  2. Don't try to pass addresses of local objects out of functions.

You wanted .child = new ChildA(), setting the child member to be a pointer to the object created by new, not a pointer to some temporary, local object. You can save the value new returns in a temporary if you want, just make sure .child gets the value new returned and not any other value.

Also:

Parent *child;

//Tried this, causes: munmap_chunk(): invalid pointer
~Container(){
    delete &child;
}

This is wrong too. What type is &child? Is &child something you got from new? This should be delete child;.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • This is a great answer to a rookie mistake. Thanks for being so thorough. My only issue now is that when I change `delete &child` to `delete child` I get: `Parent::~Parent()"is inaccessible`. In this case, yes `child` is obtained from `new`. I thought that since `child`is a pointer to a `ChildA` or `ChildB` that in order to delete it I'd need to dereference it otherwise it's just try to delete the pointer. I really stand by my walmart meat counter comment... – Matthew Goulart Feb 28 '21 at 20:12
  • @MatthewGoulart Why did you make `~Parent` inaccessible in `Container` if you want `Container` to be able to delete the `Parent` it has a pointer to? I'm confused about your objectives. Why is `~Parent` protected? – David Schwartz Feb 28 '21 at 21:58
  • I don't have control over `Parent` unfortunately. It is `JsonDocument` at the bottom of my question, part of a library that I've imported. I thought about it and it makes sense that I can't `delete` the base class as the derived classes most likely have logic specific to their deletion in the overridden destructor. – Matthew Goulart Feb 28 '21 at 22:04
  • @MatthewGoulart If the destructor isn't `virtual`, then you are correct that you can't delete the base class. That means your code for `Container` can't possibly work since it would have absolutely no way to know which destructor to invoke. Maybe use `std::unique_ptr` instead? – David Schwartz Feb 28 '21 at 22:05
  • Funny you should mention `unique_ptr`. Before the current approach, I spend a good amount of time trying to get `unique_ptr` to work, but I think it might be broken on the esp32 (or I'm just as bad at that as I am this ;) ). Either way, you've helped me realize what I'm trying is probably too complicated and I should try (yet) another approach. Thanks. – Matthew Goulart Feb 28 '21 at 22:12