Yet another heap corruption

[DonMito]

Hello,
I am having a problem with my program.

Album::Album(char *_name, Song *_songs, int _songNumber, int _salesCount, int _year)
{
        name = new char[strlen(_name) + 1];
	strcpy(name, _name);
	songNumber = _songNumber;
	songs = new Song[songNumber + 1];
	for(int i = 0; i < songNumber; i++)
	{
		songs[i] = _songs[i];
	}
	salesCount = _salesCount;
	year = _year;
}

This is my constructor for class Album.

Song::Song(char *_name, int _rating, unsigned int _duration)
{
	name = new char[strlen(_name) + 1];
	strcpy(name, _name);
	if(isRatingValid(_rating))
		rating = _rating;
	if(_duration > 0)
		duration = _duration;
}

And this is my Song consturctor. I have destructors for my Song and Album classes which delete song's name and album's name and songs. In my main I create 3 songs and put them in a album:

Song battery("Battery", 8, 312);
Song master("Master of Puppets", 10, 516);
Song thing("The Thing That Should Not Be", 5, 397);
Song songs[] = {battery, master, thing};
Album sAlbum("Master of Puppets", songs, 3, 300000, 1986);
sAlbum.print();

Everything goes ok but on return 0 it calls heap corruption error. I am trying for half a day already to find where the things break, but no success... :(

[writetonsharma]

I think you need to write a assignment operator..

songs[i] = _songs[i];

here you are creating two copies of songs but underlying pointer is only one... try to put breakpoints in destructors of the classes. more code can give more clear view.

[DonMito]

I have operator= (as well as copy-constructor)

Song& Song::operator=(const Song& s)
{ 
    if (this != &s)
    {
		del();
		copy(s);
    }
    return *this;
}

void copy(const Song& s)
{
	name = new char[strlen(s.getName() + 1)];
        strcpy(name, s.name);
	rating = s.rating;
	duration = s.duration;
}
void del()
{
	delete[] name;
	name = NULL;
}

As for the destructors

~Song(){
	del();
}
~Album(){
	delete[] name;
	delete[] songs;
	name = NULL;
	songs = NULL;
}

[cire]

Have you defined a copy constructor for Song?

[DonMito]

Yes

Song::Song(Song& s)
{
	copy(s);
}

[DonMito]

Well I debuged it with break points on all deletes. Then I removed the delete from Song's destructor and it worked. I guess I was deleting the same object twice: once in Song's destructor and once in Album's destructor - when deleting songs array

[cire]

    void copy(const Song& s)
    {
	    name = new char[strlen(s.getName() + 1)];
        strcpy(name, s.name);
	    rating = s.rating;
	    duration = s.duration;
    }

Look carefully at the argument to the strlen call here.

It should be: strlen(s.getName()) + 1

Every Song object for which copy was called resulted in name pointing to a character array that was 2 bytes short of the length it needed to be.

[DonMito]

Oh, yes... thank you :)