Programming Forums - Stash returns... Malloc problem?

Programming Forums (http://www.programmingforums.org/forumindex.php)

- C++ (http://www.programmingforums.org/forum15.html)

- - Stash returns... Malloc problem? (http://www.programmingforums.org/showthread.php?t=10004)

Stash returns... Malloc problem?

I am expanding my stash class, and I am stuck at a problem I'm having... This is my stash.h code:



//Personnal Stash beta1

//This will hold an array of characters and will return it as a string

//Not completed yet.

#ifndef STASH_H

#define STASH_H

#include <iostream>

#include <fstream>

using namespace std;



int gIncrement = 4;



class stash{

        char *ch;

        int currentSize; //position of the last character

        int currentStorage; //size of the entire array

        int next;

public:

        stash();

        stash(int startSize);

        stash(char *p);

        

        ~stash();

        

        void show(){

                cout << "currentStorage: " << currentStorage << "\n";

                cout << "current size: " << currentSize << "\n";

                cout << "Next: " << next << "\n";

                for(int i=0; i<currentSize; i++)

                        cout << ch[i];

                cout << "\n";

        }

        

        void inflate();

        void add(char p);

        void insertString(char *p);

        char* returnAsString();

        void clearStash();

        

        void operator+(char *p);

        void operator=(char *p);

        stash operator=(stash s);

        

        friend istream& operator>>(istream& stream, stash s);

        

        

};



//**constructors and destructors

stash::stash(){

        ch = new char [0];

        currentSize = 0;

        currentStorage = 0;

        next = 0;

}



stash::stash(int startSize){

        ch = new char [startSize];

        currentSize = 0;

        currentStorage = startSize;

        next = currentStorage - currentSize;

}



stash::~stash(){

        delete [] ch;

        cout << "Freeing storage\n";

}



stash::stash(char *p){

        clearStash();

        insertString(p);

}

//**



//--Increase the stash's size to hold more chars.

void stash::inflate(){

        int i;

        char *temp = new char [currentSize + gIncrement];

        for(i=0; i<currentSize; i++)

                temp[i] = ch[i];

        delete [] ch;

        ch = temp;

        currentStorage = currentStorage + gIncrement;

}



//--add a character to the stash and resize it according

//--to space needed

void stash::add(char p){

        if(next == 0)

                inflate();

        ch[currentSize] = p;

        ++currentSize;

        next = currentStorage - currentSize;

}



//--reset the stash

void stash::clearStash(){

        currentSize = 0;

        currentStorage = 0;

        ch = new char [0];

        next = currentStorage - currentSize;

}



//--insert an entire string into the stash

void stash::insertString(char *p){

        int i;

        for(i=0; p[i]; i++){

                add(p[i]);

        }

}



//--make the chars a string

char* stash::returnAsString(){

        add('\0');

        next--; //to overwrite the zero byte next time we add something

        return ch;

}



//--add more chars into the string;

void stash::operator+(char *p){

        insertString(p);

}



//--clear the stash and hold a new string

void stash::operator=(char *p){

        clearStash();

        insertString(p);

}



//--THIS HAS PROBLEMS!!!

stash stash::operator=(stash s){

        clearStash();

        insertString(s.returnAsString());

        return *this;

}



//-- Doesn't work! (NO NEED TO CHECK THIS NOW)

istream& operator>>(istream& stream, stash s){

        for(char ch = stream.get(); ch != '\n' && !stream.fail(); ch = stream.get())

                s.add(ch);

        return stream;

}

#endif

In main(), I am giving the following code:

int main(){

        stash o = "hello world";

        

        

        stash b;

        b = o;

        

        o.show();

        b.show();

        return 0;

}

But I am getting this in the console

[Session started at 2006-05-26 17:56:43 +0300.]

Freeing storage

Freeing storage

currentStorage: 12

current size: 11

Next: 1

hello world

currentStorage: 12

current size: 11

Next: 1

hello world

Freeing storage

C++ tool 3(1296) malloc: ***  Deallocation of a pointer not malloced: 0x3003c0; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug

Freeing storage

1)Why does this happen? What can I do to fix it?

2)Please if you have recommendations about my code, or any other mistakes I have done, please tell me so, I want to make this class as efficient as I can.

Thank you in advance.

Look at it this way: suppose I get a pointer to dynamic memory. Then I copy that pointer to another. Then I free the first. Then I attempt to free the second (which, as a copy of the first has already been freed).

Ah side problem, DON'T put definitions in header files. Don't let them use namespaces either.

@David, is grumpy considering putting an article on 'C++ namespaces' in your contributor's corner?

Quote:

Originally Posted by DaWei

I see what you mean. But I have a (newbish it seems) question:

Since the functions in the class do not copy the pointers, but the contents of the arrays indicated by the pointers, how does the deallocation of the 'o' object's 'ch' variable affect the 'b' object's 'ch' variable? I thought that they were a different thing.

To be more clear, I thought this thing happens: When a new stash is created, it has it's own unique 'ch' pointer variable in it. So, when I use the 'insertstring' function (which uses the 'add' function) which is used in the '=' overloaded operator, as far as I can see in my code, only the contents of the pointers are copied, and not the pointers themselves.

TheDark told me this would happen, but it seems I don't really know how to fix it.

I am cearly missing something important here. Could you provide me with more information, or at least give me a code example?

I think you just have to look at it like this: you may have two distinct variables, but they have the same content (a pointer). You can delete both variables (if they're dynamic), but deleting the thangy pointed to by the pointer can only happen once. Suppose now you have a variable with a pointer to dynamic memory. When you copy that, you make a new variable, you make a new pointer to different dynamic memory, you duplicate the contents of the first dynamic memory into the second. A true, deep copy, not a copy of a reference. Then you can delete either, or both.

@Ruben. There are a couple things I'm going to approach Grumpy about when I have my basic article publisher-formatter-swiss army knife thangy done. One would be the namespaces thing and one would be the unspecified/undefined behavior thing. I need to finish my part first, though, because otherwise the author's material becomes virtually unmaintainable. I also need to solidify my perception of what constitutes a separate article and what constitutes a piece of a FAQ/Gotchas-for-Newbies piece.

Quote:

When you copy that, you make a new variable, you make a new pointer to different dynamic memory, you duplicate the contents of the first dynamic memory into the second. A true, deep copy, not a copy of a reference. Then you can delete either, or both.

I wish I knew how to do that... I tried altering my stash.h this way:

#ifndef STASH_H

#define STASH_H

#include <iostream>

using std::cout;

using std::cin;



int gIncrement = 4;



class stash{

        char *ch;

        int currentSize; //position of the last character

        int currentStorage; //size of the entire array

        int next;

public:

        stash();

        stash(int startSize);

        stash(char *p);

        ~stash();

        

        void show(){

                cout << "currentStorage: " << currentStorage << "\n";

                cout << "current size: " << currentSize << "\n";

                cout << "Next: " << next << "\n";

                for(int i=0; i<currentSize; i++)

                        cout << ch[i];

                cout << "\n";

        }

        

        void inflate();

        void add(char p);

        void insertString(char *p);

        char* returnAsString();

        void clearStash();

        

        char returnCharacterAtPosition(int i);

        int returnCurrentSize(){return currentSize;}

        int returnCurrentStorage(){return currentStorage;}

        int returnNext(){return next;}

        

        void operator+(char *p);

        void operator=(char *p);

        void operator=(stash s);



};



//**constructors and destructors

stash::stash(){

        ch = new char [0];

        currentSize = 0;

        currentStorage = 0;

        next = 0;

}



stash::stash(int startSize){

        ch = new char [startSize];

        currentSize = 0;

        currentStorage = startSize;

        next = currentStorage - currentSize;

}



stash::~stash(){

        delete [] ch;

        cout << "Freeing storage\n";

}



stash::stash(char *p){

        clearStash();

        insertString(p);

}

//**



//--Increase the stash's size to hold more chars.

void stash::inflate(){

        int i;

        char *temp = new char [currentSize + gIncrement];

        for(i=0; i<currentSize; i++)

                temp[i] = ch[i];

        delete [] ch;

        ch = temp;

        currentStorage = currentStorage + gIncrement;

}



//--add a character to the stash and resize it according

//--to space needed

void stash::add(char p){

        if(next == 0)

                inflate();

        ch[currentSize] = p;

        ++currentSize;

        next = currentStorage - currentSize;

}



//--reset the stash

void stash::clearStash(){

        currentSize = 0;

        currentStorage = 0;

        ch = new char [0];

        next = currentStorage - currentSize;

}



//--insert an entire string into the stash

void stash::insertString(char *p){

        int i;

        for(i=0; p[i]; i++){

                add(p[i]);

        }

}



//--make the chars a string

char* stash::returnAsString(){

        add('\0');

        next--; //to overwrite the zero byte next time we add something

        return ch;

}



//--add more chars into the string;

void stash::operator+(char *p){

        insertString(p);

}



//--Return a character at a specified position

//--Return NULL if a character doesn't exist at the

//--requested position

char stash::returnCharacterAtPosition(int i){

        if(i>currentSize)

                return NULL;

        return ch[i];

}



//--clear the stash and hold a new string

void stash::operator=(char *p){

        

        clearStash();

        insertString(p);

}



//--THIS HAS PROBLEMS WITH MEMORY ALLOCATION!!!

void stash::operator=(stash s){

        int i;

        delete [] ch;

        char *temp = new char [s.returnCurrentSize()];

        for(i=0; i<s.returnCurrentSize(); i++){

                temp[i] = s.returnCharacterAtPosition(i);

        }

        next = s.returnNext();

        currentStorage = s.returnCurrentStorage();

        currentSize = s.returnCurrentSize();

        

        ch = temp;

}



#endif

I get this result:

Quote:

[Session started at 2006-05-26 19:55:16 +0300.]
Freeing storage
currentStorage: 12
current size: 11
Next: 1
hello world
currentStorage: 12
current size: 11
Next: 1
hello world
Freeing storage
C++ tool 3(1788) malloc: *** Deallocation of a pointer not malloced: 0x3003b0; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug
Freeing storage

C++ tool 3 has exited with status 0.

which is just like before.

Dawei, could you (or anyone else) provide me with a simple code example of what should I do (not the whole code) because I really don't seem to be able to fix this annoying thing.

This is why you need a copy constructor. Let me sort of lay it out, and if that doesn't work, I'll write some code. Say you have a class with a variable that holds a reference to memory you have gotten from 'new'. If you just copy the contents of that class you have a different variable, but its contents are the same pointer to the same new'ed material. You cannot free the memory referenced by that pointer but once. Suppose now you write a copy constructor. You have a new class and it has a variable just like the old one. You do not copy the contents of that variable, because you just get the same reference. Instead of doing that, you do exactly what you did for the original: you get a fresh set of bytes from 'new'. Then you copy what was referenced by the old variable into the new memory, and put the pointer to that in the new variable, instead of a copy of the old pointer. I'm afraid I'm not being clear. Look at these images representing the original, a shallow copy, and a deep copy. I apologize for the huge images, they didn't reduce well. (Bite me, Ruben.)
http://www.daweidesigns.com/images/class.gif
-----------------------
http://www.daweidesigns.com/images/shallow.gif
-----------------------
http://www.daweidesigns.com/images/deepcopy.gif

Dawei, thank you VERY much. I solved the problem. Here is my code:

#ifndef STASH_H

#define STASH_H

#include <iostream>

using std::cout;

using std::cin;



int gIncrement = 4;



class stash{

        char *ch;

        int currentSize; //position of the last character

        int currentStorage; //size of the entire array

        int next;

public:

        stash();

        stash(int startSize);

        stash(char *p);

        stash(stash& other);//copy constructor

        ~stash();

        

        void show(){

                cout << "currentStorage: " << currentStorage << "\n";

                cout << "current size: " << currentSize << "\n";

                cout << "Next: " << next << "\n";

                for(int i=0; i<currentSize; i++)

                        cout << ch[i];

                cout << "\n";

        }

        

        void inflate();

        void add(char p);

        void insertString(char *p);

        char* returnAsString();

        void clearStash();

        

        char returnCharacterAtPosition(int i);

        int returnCurrentSize(){return currentSize;}

        int returnCurrentStorage(){return currentStorage;}

        int returnNext(){return next;}

        char* returnPointerToCh(){return ch;}

        

        void operator+(char *p);

        void operator=(char *p);

        void operator=(stash &s);

        

};



//**constructors and destructors

stash::stash(){

        ch = new char [0];

        currentSize = 0;

        currentStorage = 0;

        next = 0;

}



stash::stash(int startSize){

        ch = new char [startSize];

        currentSize = 0;

        currentStorage = startSize;

        next = currentStorage - currentSize;

}



stash::~stash(){

        delete [] ch;

        cout << "Freeing storage\n";

}



stash::stash(char *p){

        clearStash();

        insertString(p);

}



stash::stash(stash& other){

        clearStash();

        insertString(other.returnPointerToCh());

}



//**



//--Increase the stash's size to hold more chars.

void stash::inflate(){

        int i;

        char *temp = new char [currentSize + gIncrement];

        for(i=0; i<currentSize; i++)

                temp[i] = ch[i];

        delete [] ch;

        ch = temp;

        currentStorage = currentStorage + gIncrement;

}



//--add a character to the stash and resize it according

//--to space needed

void stash::add(char p){

        if(next == 0)

                inflate();

        ch[currentSize] = p;

        ++currentSize;

        next = currentStorage - currentSize;

}



//--reset the stash

void stash::clearStash(){

        currentSize = 0;

        currentStorage = 0;

        ch = new char [0];

        next = currentStorage - currentSize;

}



//--insert an entire string into the stash

void stash::insertString(char *p){

        int i;

        for(i=0; p[i]; i++){

                add(p[i]);

        }

}



//--make the chars a string

char* stash::returnAsString(){

        add('\0');

        next--; //to overwrite the zero byte next time we add something

        return ch;

}



//--add more chars into the string;

void stash::operator+(char *p){

        insertString(p);

}



//--Return a character at a specified position

//--Return NULL if a character doesn't exist at the

//--requested position

char stash::returnCharacterAtPosition(int i){

        if(i>currentSize)

                return NULL;

        return ch[i];

}



//--clear the stash and hold a new string

void stash::operator=(char *p){

        

        clearStash();

        insertString(p);

}



//--THIS HAS PROBLEMS WITH MEMORY ALLOCATION!!!

void stash::operator=(stash &s){

        int i;

        delete [] ch;

        char *temp = new char [s.returnCurrentSize()];

        for(i=0; i<s.returnCurrentSize(); i++){

                temp[i] = s.returnCharacterAtPosition(i);

        }

        next = s.returnNext();

        currentStorage = s.returnCurrentStorage();

        currentSize = s.returnCurrentSize();

        

        ch = temp;

}



#endif

If you have noticed anything else that needs fixing or optimization, say it! :). Please check the '=' overloaded operators, and the copy constructor, because I may have missed a detail there, although they seem to work fine...

Like I said, use header files correctly.
I'd also use C++ strings, but I don't know if you are prohibited to do that or anything. C++ strings have their own memory management, which is ideal.

main.cpp:

#include "stash.h"



int main()

{

        stash o("hello world");

        stash b;

        b = o;



        o.show();

        b.show();

        return 0;

}

stash.cpp

#include "stash.h"



int gIncrement = 4;



//**constructors and destructors

stash::stash(){

        ch = new char [0];

        currentSize = 0;

        currentStorage = 0;

        next = 0;

}



stash::stash(int startSize){

        ch = new char [startSize];

        currentSize = 0;

        currentStorage = startSize;

        next = currentStorage - currentSize;

}



stash::~stash(){

        delete [] ch;

        std::cout << "Freeing storage\n";

}



stash::stash(char *p){

        clearStash();

        insertString(p);

}



stash::stash(stash& other){

        clearStash();

        insertString(other.returnPointerToCh());

}



//**



//--Increase the stash's size to hold more chars.

void stash::inflate(){

        int i;

        char *temp = new char [currentSize + gIncrement];

        for(i=0; i<currentSize; i++)

                temp[i] = ch[i];

        delete [] ch;

        ch = temp;

        currentStorage = currentStorage + gIncrement;

}



//--add a character to the stash and resize it according

//--to space needed

void stash::add(char p){

        if(next == 0)

                inflate();

        ch[currentSize] = p;

        ++currentSize;

        next = currentStorage - currentSize;

}



//--reset the stash

void stash::clearStash(){

        currentSize = 0;

        currentStorage = 0;

        ch = new char [0];

        next = currentStorage - currentSize;

}



//--insert an entire string into the stash

void stash::insertString(char *p){

        int i;

        for(i=0; p[i]; i++){

                add(p[i]);

        }

}



//--make the chars a string

char* stash::returnAsString(){

        add('\0');

        next--; //to overwrite the zero byte next time we add something

        return ch;

}



//--add more chars into the string;

void stash::operator+(char *p){

        insertString(p);

}



//--Return a character at a specified position

//--Return NULL if a character doesn't exist at the

//--requested position

char stash::returnCharacterAtPosition(int i){

        if(i>currentSize)

                return NULL;

        return ch[i];

}



//--clear the stash and hold a new string

void stash::operator=(char *p){



        clearStash();

        insertString(p);

}



//--THIS HAS PROBLEMS WITH MEMORY ALLOCATION!!!

void stash::operator=(stash &s){

        int i;

        delete [] ch;

        char *temp = new char [s.returnCurrentSize()];

        for(i=0; i<s.returnCurrentSize(); i++){

                temp[i] = s.returnCharacterAtPosition(i);

        }

        next = s.returnNext();

        currentStorage = s.returnCurrentStorage();

        currentSize = s.returnCurrentSize();



        ch = temp;

}

stash.h

#ifndef STASH_H

#define STASH_H



#include <iostream>



class stash{

        char *ch;

        int currentSize;

        int currentStorage;

        int next;

public:

        stash();

        stash(int startSize);

        stash(char *p);

        stash(stash& other);

        ~stash();



        void show(){

                std::cout << "currentStorage: " << currentStorage << std::endl;

                std::cout << "current size: " << currentSize << std::endl;

                std::cout << "Next: " << next << std::endl;

                for(int i=0; i<currentSize; i++)

                        std::cout << ch[i];

                std::cout << std::endl;

        }



        void inflate();

        void add(char p);

        void insertString(char *p);

        char* returnAsString();

        void clearStash();



        char returnCharacterAtPosition(int i);

        int returnCurrentSize(){return currentSize;}

        int returnCurrentStorage(){return currentStorage;}

        int returnNext(){return next;}

        char* returnPointerToCh(){return ch;}



        void operator+(char *p);

        void operator=(char *p);

        void operator=(stash &s);



};



#endif

A side question: I see Soulstorm doing:

void stash::clearStash(){

        currentSize = 0;

        currentStorage = 0;

        ch = new char [0];

        next = currentStorage - currentSize;

}

So he doesn't delete [] anything, will new do a resize next time it's called (like realloc)?

So then the following would function correctly:

int main()

{

        char * ch = new char[5];

        ch = new char[30];

        ch = new char[1];

        delete [] ch;



        return 0;

}