Dynamic array malfunction.

[Niels Meijer]

Dear everyone,
I want to make a little music playing program that can read notes from a file. I already made this program, but I coded it without preparation and testing while coding, and a unsolvable problem occured. I remade the whole program and tested it all the time. I wrote this piece of code, but it seems that it has some difficulties with the pseudo-multidimensional (it actually is a array of arrays instead of a 'real' 2D C++ array, because you cannot allocate a real multidimensional array) dynamic array which preserves the notes. the clog statements show that the other values are ok, but after the program writes two (wrong) contents of the array mentioned above, it crashes. Perhaps I need to clean (set all the variables to 0) the array before using it, but I assign values to it before using, so I don't think that is necessary.

#include<iostream>
struct Song
{
  Song(unsigned char**SA,
       unsigned char BPM,
	   unsigned char MSB,
	   unsigned long long SL);
  ~Song();
  unsigned char**arr;
  unsigned char BPM,maxSynBars;
  unsigned long long songLength;
};
Song::Song(unsigned char**SA,
       unsigned char BPM,
	   unsigned char MSB,
	   unsigned long long SL):
  arr(SA),BPM(BPM),maxSynBars(MSB),songLength(SL)
{
  for(unsigned char x=0;x<MSB;x++)
  {
  	arr[x]=new unsigned char[SL];
  	for(unsigned long long y=0;y<SL;y++)
  	  arr[x][y]=SA[x][y];
  }
}
Song::~Song()
{
  for(unsigned i=0;i<maxSynBars;i++)
    delete[] arr[i];
  delete[] arr;
}
int main()
{
  unsigned char**songArr=new unsigned char*[2];
  songArr[0]=new unsigned char[6];
  songArr[1]=new unsigned char[6];
  songArr[0][0]=45;
  songArr[0][1]=48;
  songArr[0][2]=43;
  songArr[0][3]=48;
  songArr[0][4]=41;
  songArr[0][5]=41;
  songArr[1][0]=41;
  songArr[1][1]=84;
  songArr[1][2]=40;
  songArr[1][3]=84;
  songArr[1][4]=38;
  songArr[1][5]=38;
  Song*s=new Song(songArr,160,2,6);
  std::clog<<"BPM:\n"
  <<static_cast<unsigned>(s->BPM)<<'\n'
  <<"maxSynBars:\n"
  <<static_cast<unsigned>(s->maxSynBars)<<'\n'
  <<"songLength:\n"
  <<s->songLength<<'\n';
    std::clog<<"arr:\n";
  for(unsigned char x=0;x<s->maxSynBars;x++)
  {
    for(unsigned long long y=0;y<s->songLength;x++)
      std::clog<<static_cast<unsigned>(s->arr[x][y])<<' ';
    std::clog<<'\n';
  }
  delete s;
  delete[] songArr[0];
  delete[] songArr[1];
  delete[] songArr;
}

WHAT IN THE WORLD IS WRONG WITH THIS CODE!?!?!
I don't get it!

Kind regards,
Niels Meijer

[Peter87]

You are deleting the songArr twice. Once in main and once in the Song destructor.

On line 59 you are incrementing x instead of y.

[Niels Meijer]

Ah, thanks for the tip about the increment! I am deleting songArr because s has a copy of songArr, and it doesn't make any difference...
Well, I am able to access arr (in s), but this shows up if I run the program:

BPM:
160
maxSynBars:
2
songLength:
6
arr:
168 17 141 0 196 0
168 17 141 0 196 0

this should be:

BPM:
160
maxSynBars:
2
songLength:
6
arr:
45 48 43 48 41 41
41 84 40 84 38 38

arr doesn't contain the desired values, any idea what's wrong? I can't believe it's my algorithm because I remade it twice. Am I stumbling on my limited C++ knowledge again?

thanks for your answer,
Niels Meijer

[cire]

Song::Song(unsigned char**SA,
       unsigned char BPM,
	   unsigned char MSB,
	   unsigned long long SL):
  arr(SA),BPM(BPM),maxSynBars(MSB),songLength(SL)
{
  for(unsigned char x=0;x<MSB;x++)
  {
  	arr[x]=new unsigned char[SL];
  	for(unsigned long long y=0;y<SL;y++)
  	  arr[x][y]=SA[x][y];
  }
}

arr is given the value held by SA. That is, arr points to the same memory that SongArr points to in main. That is, arr does not point to a copy of the contents of SongArr; it points to the same memory.

What you do is make each element of SongArr (in it's guise as Song::arr) point to a different place in memory (via new and assignment in the constructor.) Then, you assign the content of that new memory to the... content of the new memory.

[Niels Meijer]

So array1[x][y]=array2[x][y]; doesn't mean
'change the value in array1 on place x,y to the value in array2 on place x,y'
but
'set the location of the value in array1 on place x,y to the location of the value in array2 on place x,y'
isn't it?
If so, how do I tell the compiler the first message? if I use new I'll still have to use the statement array1[x][y]=array2[x][y];. Please explain, I'm experienced in C++, but dynamic memory just isn't my strongest point...

Kind regards,
Niels Meijer

[cire]

So array1[x][y]=array2[x][y]; doesn't mean
'change the value in array1 on place x,y to the value in array2 on place x,y'

It means that alright. But when array1 and array2 point to the same place in memory, array1[x][y] = array2[x][y]; is the same as array1[x][y] = array1[x][y], which is essentially a no-op.

If you replace arr(SA) with arr(new unsigned char*[MSB]) you should be in business, but you're using a lot of dynamic memory allocations where you don't really need to which makes the code a lot hairier than it should be.

[Niels Meijer]

I thought I removed the arr(SA) statement, but it seems I didn't...
It works now, next time I should check my code better.
Thanks for your replies!