Access violation writing location in C++

Question

First time writing here. I've seen several other questions like mine, but couldn't solve my problem. I have 4 classes : Song, Playlist, Album, Artist. Playlist consist of array of songs. Album inherits Playlist and has some more functuality and Artist consist of array of Albums and some other stuff.

Album(char* _name, Song* songs,int _sales ,int _year,int n):Playlist(songs,n)
    {
        name = new char[strlen(_name)+1];
        strcpy(name,_name);
        salesCount=_sales;
        year = _year;
    };
Playlist::Playlist(Song* _songlist, int n)
{
    if(n > 20)
    {
        cout<<"The number of song must be lesser or equal to 20"<<endl;
        return;
    } 

    SongList = new Song[n];
    for(int i = 0 ; i<n; i++)
    {
        SongList[i] = _songlist[i];
    }
numberOfSongs=n;
}

When I create an Album using this constructor I don't have anyproblems, but when I try to create an Array of albums , that consists of album created with this construtor I get a memory access violation. Any Ideas ?

Ok so I updated my code a little bit. Here is what I'm trying to do in the main method:

Song newSong("My Song", 9, 231),mySong("Your Song", 8 , 180),yourSong("His Song",7,135), herSong("Her Song",8,431);
Song songs[4] = {newSong, mySong,yourSong,herSong};
Album yourAlbum("My Album",songs,200000, 2010, 4);
yourAlbum.PrintInfo(); //<------- WORKS
Album albums[1]; //<------ It calls the deffault constructor of Album and after it gives me //"Source not available"
//When I try to declare it like this Album albums[1] = { yourAlbum } ; it gives me access violation

Artist myArtist("blake", albums);
myArtist.PrintArtistInfo();

Use `std::string` instead of `char*` and `std::vector` instead of dynamically allocating `Song*` arrays. It will make life considerably simpler. Also http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — hmjd, Mar 27 '13 at 14:19
@roger_rowland It is simple as this. When I have for example: `code` Album a; Album b; Album albums[2] = {a,b}; `code` I have no problems, But when I try to do `code'Album myalbum("name", songs,200000,2010); Album albums[1] = {myalbum};`code` I have a memory access violation — VMM, Mar 27 '13 at 14:23
@user2215925 http://stackoverflow.com/questions/1461432 This is what's going wrong for you. — Drew Dormann, Mar 27 '13 at 14:25
@fatih_k Yes, I have a defined deffault constructor for class Album — VMM, Mar 27 '13 at 14:34
Maybe I'm lagging horribly behind with the latest C++ standards, but this code seems like random pseudo code goo to me. Does this really compile? — Lundin, Mar 27 '13 at 14:39
@Lundin those are just two constructos of 600 lines of code. — VMM, Mar 27 '13 at 14:41

LihO · Accepted Answer · 2013-03-27T14:34:14.093

4

Playlist::Playlist(Song* _songlist) {
    numberOfSongs = sizeof(*_songlist)/sizeof(_songlist);  // <-- THIS
    ...

leads to unexpected (nonsense) value being assigned to the numberOfSongs since it is equal to:

numberOfSongs = sizeof(Song) / sizeof(Song*);

In case you need your function to know the size of the dynamically allocated array, you should keep track of this size on your own and pass it to this function.

Also note that since you are programming in C++, you should use features that this language provides,. You should use std::string instead of C-style strings and STL containers such as std::vector instead of C-style arrays (in case of std::vector, the object internally holds the information about its length, so you wouldn't be dealing with this kind of problems).

edited Mar 27 '13 at 14:34

answered Mar 27 '13 at 14:20

LihO

37,789
9
89
156

The funny thing is that it works. I completely agree for the `std::string` , but this is for a university project and we have to practice other techniques. What amezes me most is that when I create an album using this constructor I can use all of the functuality that the class has, but I canot add it to an array – VMM Mar 27 '13 at 14:30
@user2215925: I shouldn't have written "not work", it's so misleading and confusing. I edited my answer :) – LihO Mar 27 '13 at 14:35

Agentlien · Answer 2 · 2013-03-27T14:59:22.340

The problem here is that you are allocating an array of sizeof(*_songlist)/sizeof(_songlist); elements.

That is, the number of Song items you are allocating is equal to the number of bytes used to store a Song, divided by the number of bytes used to store a pointer (to a Song). This is a rather meaningless value.

Furthermore, this number is a constant value which doesn't depend on the number of songs in the array you passed it. Hence, any run of this function will always allocate the same amount of memory, regardless of what it is passed. If this number is larger than the number of songs actually passed, you will be indexing outside of the array, which is undefined behaviour. That would explain your segmentation fault.

If, on the other hand, you pass more songs than this constant, you will initialize your array with the subset of songs which fits into the array you allocated, missing some data. To solve this problem, either pass a (reference to a) std::vector of songs or add an extra argument containing the actual number of songs in the array and use that as the size of the array.

I see, I have tested it only with tree songs and it worked properly. I will pass it as a value and see what happens. — VMM, Mar 27 '13 at 14:50
Could you update your example to include how you are calling the Playlist constructor? — Agentlien, Mar 27 '13 at 14:57

Access violation writing location in C++

2 Answers2