CODEWITHSUNDEEP
c++
bugboy
bugboy

Reputation:

delete [] char *, memory issues

I have a global pointer variable

char* pointer = new char[500];
/* some operations... */

there is a seperate FreeGlobal() function that does free up the pointer as below:

delete[] pointer;

First time when the function is called, it actually frees up the memory and now the pointer is a bad pointer. But when we call this more than once, it throws an exception.

Is there a way to check the pointer variable before calling delete [] again? What are the work arounds? Is this a bad practice?

Thank you.

Upvotes: 6

Views: 23043

Answers (7)

jkeys
jkeys

Reputation: 3955

Your freeGlobal() function should make it point to 0, so define the function as such:

//if ptr is not null, delete; otherwise, return
void freeGlobal(char*& ptr)
{
   if (ptr != 0)
   {
      delete[] ptr;
      ptr = 0;
   }
}

Edit: 0 == NULL, at least for now, in C++.

Upvotes: 1

David Andres
David Andres

Reputation: 31781

Set pointer to null after you delete it. You should not try to delete the same data more than once.

As mentioned by GRB in the comments for this post, it is perfectly safe to call delete[] NULL.

Upvotes: 15

GManNickG
GManNickG

Reputation: 503825

I would suggest you actually fix your code. Double deleting something is a terrible thing to do. Rather than make hackish routines to let this happen, fix the real problem: no more double deletes.

Find out why you're deleting something twice, and stop it from happening. This would probably be easier if you weren't using global variables. If you need a global resource, use a singleton.

Additionally, use a std::vector<char>, or boost::array<char, 100>, so you don't need to worry about memory. You can't accidentally delete something twice if you don't have to worry about (or access to) deleting it.

Upvotes: 5

jmucchiello
jmucchiello

Reputation: 18984

You don't need to check if the pointer is null before calling delete or delete[]. Your function should look like this:

void freeGlobal(char*& ptr) {
    delete[] ptr;
    ptr = 0;
}

What I want to know is why this thing is global and not inside a class?

class buffer {
    char* buf;
    size_t size;
public:
    buffer(size_t n) : size(n), buf(new char[size]) {}
    ~buffer() { delete[] buf; buf = 0; }

    operator char*() { return buf; }
    char& operator[](size_t ofs) {
        assert(ofs >= 0 && ofs < size);
        return buf[ofs];
    }
};

There are probably a couple things wrong with my implementation as I just typing it in here. Another question is why you aren't using the std::string for this char buffer?

Upvotes: 3

Michael Kristofik
Michael Kristofik

Reputation: 35188

delete on a bad pointer results in Undefined Behavior. You're lucky that it threw an exception, but you certainly shouldn't rely on that happening. To prevent it, simply set the pointer to 0 after the delete. It's safe to delete a null pointer (in case FreeGlobal() gets called more than once), so you don't have to do any if-test.

void FreeGlobal()
{
    delete [] pointer;
    pointer = 0;
}

Upvotes: 3

KFro
KFro

Reputation: 764

Check this link out at the CERT Coding Standard Site. After freeing a variable you should set the pointer back to NULL since its not really pointing to anything anymore.

Upvotes: 1

Havenard
Havenard

Reputation: 27854

Pointers pointing to nowhere, should point to nowhere. pointer = null;

if (pointer != null) {
delete[] pointer;
pointer = null;
}

Upvotes: 0

Related Questions