using "new" and "delete" inside class function-CodePudding

I want to be sure that using new and delete to free heap memory is done as needed.

Following function returns a char *. Inside each function I use new for the returned value, and I delete afterwards.

Is it the right way to free heap memory for function return?

const char *myIOT2::_devName()
{
    char *ret = new char[MaxTopicLength2];
    if (strcmp(addGroupTopic, "") != 0)
    {
        snprintf(ret, MaxTopicLength2, "%s/%s/%s", prefixTopic, addGroupTopic, deviceTopic);
    }
    else
    {
        snprintf(ret, MaxTopicLength2, "%s/%s", prefixTopic, deviceTopic);
    }
    return ret;
}
const char *myIOT2::_availName()
{
    char *ret = new char[MaxTopicLength2];
    const char *DEV = _devName();
    snprintf(ret, MaxTopicLength2, "%s/Avail", DEV);
    delete DEV;
    return ret;
}

To point out: the fact the I DEV: const char *DEV = _devName(); in order to use it as a parameter in snprintf(ret, MaxTopicLength2, "%s/Avail", DEV); just to be able to delete it later as delete DEV; - is this correct?

CodePudding user response：

At least as I see things, you really only have two sane choices here. One is for the caller to handle all the memory management. The other is for the callee to handle all the memory management.

But what you're doing right now (callee handles allocation, caller handles de-allocation) is a path to madness and memory leaks.

If the caller is going to manage the memory, this all becomes fairly simple:

const char *myIOT2::_devName(char *ret, size_t maxlen)
{
    if (strcmp(addGroupTopic, "") != 0)
    {
        snprintf(ret, maxlen, "%s/%s/%s", prefixTopic, addGroupTopic, deviceTopic);
    }
    else
    {
        snprintf(ret, maxlen, "%s/%s", prefixTopic, deviceTopic);
    }
    return ret;
}

If the callee is going to handle all the memory management, you'd normally use std::string. Since you're on an Arduino, however, std::string isn't available, and you need to use their own String class instead. Either way, you simply allocate a String object and put your contents into it. It takes care of the actual memory allocation to hold the contents, and will free its contents when the String object is destroyed.

Given the small amount of memory normally available on an Arduino, having the caller allocate the memory is usually going to work out better. But (especially if this is something that doesn't happen very often, so you won't run into heap fragmentation problems) allocating space on the heap can work reasonably well also.

But I'll repeat: trying to mix memory management so the callee allocates and the caller deletes...is the stuff of nightmares. When you read about C circa 1993, and hear about lots of problems with memory leaks...this is exactly the sort of thing that led to them.

CodePudding user response：

ret allocated memory:

const char *myIOT2::_devName()
{
    char *ret = new char[MaxTopicLength2];
    return ret;
}

And below, you'll see that by deleting DEV, you will free the memory, because it's actually ret. But this time, you should remove ret2 somewhere else since it's the return value and there's no way to delete it inside that scope:

const char *myIOT2::_availName()
{
    char *ret2 = new char[MaxTopicLength2];
    const char *DEV = _devName();
    delete[] DEV;
    return ret2;
}

Also, note the following:

char* str = new char [30]; // Give str a memory address.

// delete [] str; // Remove the first comment marking in this line to correct.

str = new char [60]; /* Give str another memory address with
                                                    the first one gone forever.*/

delete [] str; // This deletes the 60 bytes, not just the first 30.