Shouldn't I free the memory I malloced in constructors?

I have this inside my constructor:

Code:

	ippFile = NULL;
	fileName = (char*) malloc(strlen(name));
	strcpy(fileName,name);

And this in my destructor:

Code:

free(fileName);

When that destructor is called I get the following message:

Code:

Microsoft Visual C++ Debug Library
--------------------------------------------
Debug Error!

Program: xxxxxxx.exe

DAMAGE: after Normal block (#41) at 0x003F0860


(Press Retry to debug the aplication)

Now. If I don't do that free, everything runs smoothly... What is going on?

> fileName = (char*) malloc(strlen(name));
1. You didn't count the \0, so the strcpy trashed some memory
2. Use new/delete for allocating memory in C++

strlen(name)+1 is how many you need.

fileName = new char[strlen(name) + 1];



Like that, right?

Yes

And
delete [ ] fileName;
in the destructor.

There's no excuse for not using std::string for this.

there is of course. i'm not gonna explain that. there could be plenty of reasons for someone to prefer a normal string.

Originally Posted by pronecracker

there is of course. i'm not gonna explain that. there could be plenty of reasons for someone to prefer a normal string.

Sure.. But maybe at least use new instead of malloc? Unless you know you're going to be using realloc() later, I'd say use new.

>> there could be plenty of reasons for someone to prefer a normal string.

Unfortunately the most common reasons are based on the antiquated notion that a null terminated character array is a normal string in C++. The C++ string class is the closest thing to a normal string in C++, and in many cases (especially on forums such as these) the excuses for using C style are poor.

My excuse is that I'm a C programmer who has started using C++ a week ago...

That's a better excuse than most. Although if you really want to learn C++ I'd suggest learning the C++ string class (and vector and other standard C++ library tools).

Originally Posted by brewbuck

Sure.. But maybe at least use new instead of malloc? Unless you know you're going to be using realloc() later, I'd say use new.

I would like to mention that even then it isn't necessary to use C functions. It's even less necessary to do this at all I suppose, but

Code:

#include <iostream>
#include <cstdlib>
#include <ctime>

void print( int base[], std::size_t size )
{
    for( std::size_t i = 0; i < size; ++i )
    {
        std::cout << ' ' << base[ i ];
    }
    std::cout << std::endl;
}

int main( )
{
    const std::size_t ORIG_SIZE = 4;
    const std::size_t NEW_SIZE = 8;

    // 1) Build an array of some size.
    int *bar = new int[ ORIG_SIZE ];
    std::time_t now;

    std::time( &now );
    std::srand( now );

    // 2) Use the array however you like.
    for( std::size_t i = 0; i < ORIG_SIZE; ++i )
        bar[ i ] = std::rand( );

    std::cout << "Before: ";
    print( bar, ORIG_SIZE );

    // 3) Make a temporary array.
    int *temp_bar = new int[ ORIG_SIZE ];
    // 4) Copy objects over to this array.
    for( std::size_t i = 0; i < ORIG_SIZE; ++i )
        temp_bar[ i ] = bar[ i ];

    // 5) Extend the array with new after deleting the old array data.
    delete [] bar;
    bar = new int[ NEW_SIZE ];
    // 6) Refill the extended array.
    for( size_t i = 0; i < NEW_SIZE; ++i )
        bar[ i ] = ( i < ORIG_SIZE )? temp_bar[ i ] : std::rand( );

    delete [] temp_bar;

    std::cout << "After : ";
    print( bar, NEW_SIZE );
    // and so on ...

    // 7) Clean up when you're done!
    delete [] bar;
}

/*
* My output:
*
* Before:  24197 24557 21828 14805
* After :  24197 24557 21828 14805 20141 15933 21001 9472
*/

Sorry about the bump for this...