Deleting pointer to const char * - pointer being freed was not allocated

[afedorov]

When I run following code:

class A {
    const char *text;
public:
    A(const char *text) : text(text) {}

    virtual ~A() {
        delete[] text;
//  the same result with  delete text;
    }
};


int main() {
    A *a = new A("Hello world");
    delete a;
    return 0;
}

I got

Research(69474,0x7fff74090000) malloc: *** error for object 0x10377cf90: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

How can I properly remove text pointer

[Repeater]

You should only call delete on a pointer that was returned from a call to new.

The pointer text at no point is such a pointer. You should not be calling delete on text.

[Peter87]

delete should only be used on things that has been created with new. A string literal exists for the whole duration of the program and can't be deleted.

What you might want to do in your constructor is to create a copy of the string. Otherwise you will have problems if the string that is passed to the constructor is not a string literal because it could be destroyed before the class object is destroyed.

Note that all this becomes much easier if you use std::string. Then you don't need to worry about new, delete, and pointers inside the class.

[Duthomhas]

You have two pointers in your code:

Line  2: const char* text;
Line 14:A *a = ...

You assign a dynamically-allocated value to a, and properly clean up.

But you assign a non-dynamically-allocated, global string literal constant to text ...and then try to delete[] it. The runtime properly barfs.


If you intend to accept string literals as assignment as delete-able resources, you must make a proper copy.

#include <cstring>

char* clone( const char* s )
{
  char* r = new char[ std::strlen( s ) + 1 ];
  return r ? std::strcpy( r, s ) : NULL;
}

class A {
    const char *text;
public:
    A(const char *text) : text(text) {}

    virtual ~A() {
        delete[] text;
    }
};


int main() {
    A *a = new A(clone("Hello world"));
    delete a;
    return 0;
}

That said, you would do better to avoid dealing with this stuff yourself. Use std::string:

#include <memory>
#include <string>

class A {
    std::string text;
public:
    A(const std::string& text) : text(text) {}

    virtual ~A() {
    }
};


int main() {
    std::shared_ptr<A> a = std::make_shared<A>("Hello world");
    return 0;
}

Hope this helps.

[edit]
Sorry, the std::string means you don't have to clean up after the text, and can treat it like normal.

The std::shared_ptr means you get an object you can treat like a normal pointer-to-A, but you don't have to clean up yourself.

  auto a = std::make_shared<A>("Hello world!");
  ...
  a->do_something();
  ...
  return 0;

Hope that helps better. :)

[edit 2]
Accidentally used std::strdup() and delete[] together ...which was dumb. Fixed.

[Repeater]

At risk of being that awkward guy on a forum, doesn't strdup demand free rather than delete?

I stand by to be corrected; I certainly have a memory of that sort of POSIX C library function not being exactly immutable throughout the years.

[Duthomhas]

Heh, lol, you're right. Let me fix that.

[afedorov]

I will use similar code in Arduino platform where I don't have access to std functions,

The pointer text at no point is such a pointer. You should not be calling delete on text.

Does it mean that memory used by text will be cleaned automatically?

[jonnin]

this isnt allocating dynamic memory.

const char * cp = "no memory allocated here";
delete[] cp; //error

you don't need to do anything for this type of c-string.

[afedorov]

basically, pointers are always copied by value. Is it?

So instance of A will have it's own copy of the text variable.

[Repeater]

So instance of A will have it's own copy of the text variable.

Every instance of A will have it's own variable named "text". If you create 10 instances of object A, you create 10 variables named text.

Each of those variables named text, being a pointer, may point at the same piece of memory, but there are ten instances of variables named text.

basically, pointers are always copied by value. Is it?

There's nothing special about them. You can copy them by value, just like any other variable. You can copy them by reference, just like any other variable.

[afedorov]

But in this case if I have function like this

A *getA() {
    const char *text = "ASDF";
    A *a = new A(text);
    return a;
}

instance of text will be delete after function is done and field in A will point to garbage. Correct?

[Cubbi]

The instance of text may be gone, but "ASDF" is a static array, it is forever (it exists from the moment the program is loaded into memory by the OS, until the program is terminated)

And really don't call 'new' in a C++ program, this isn't 1992: auto getA() { return std::make_unique<A>("ASDF"); } - see https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-new

[afedorov]

Thank you.

I used new just as an example. I have to (I couldn't figure out better way) to use new in my real project as I have polymorphic structures, and C++ allows only pointers for abstract classes. As I have to pass objects around I need to allocate this memory so I don't loose objects when function is done.

[keskiverto]

Yes, polymorphism sets some requirements.

Q: Does the code below
1. Use new
2. Have any pointers
3. Clean itself

auto a = std::make_shared<A>("Hello world!");
auto b = std::make_unique<A>("Hello world!");
std::string c("Hello world!"); 

[afedorov]

:) I will use Arduino platform for my project that doesn't support those features.

Any suggestions for good old C++ without std?

[mbozzi]

I will use Arduino platform for my project that doesn't support those features

Consider writing your own (naive) version of std::unique_ptr.

[Thomas1965]

Without smart pointer have have to use the bad old raw pointers.
Store them in a vector inside a class and delete them in the destructor.
What does Arduino actually support?

[Mridula]

Delete is used to free the memory for a C++ class object. It should not be called on a text. Moreover delete is used to free the memory of objects created by using "new" keyword.

[Cubbi]

Thomas1965 wrote:

What does Arduino actually support?

Arduino Programming language, loosely based on (some small part of) C++: https://www.arduino.cc/reference/en/ (plus whatever "avr-gcc" supports, which apparently excludes a whole lot)

[jonnin]

We usually just went to 90's procedural and very simple classes for the platform. The systems are small memory and the language is aggravating with a number of nonstandard (esp around string handling) functions and missing things (and this me saying it, with my limited knowledge of c++ 11 and above!!).