CODEWITHSUNDEEP
c++arraysoverwrite
Tom Spargo
Tom Spargo

Reputation: 59

How are my array values getting overwritten

I am creating an array of ints and an instance of a class that overloads the () operator. Here's the class:

template class CMatrix { public:

CMatrix(const int d);
CMatrix(const int d1, const int d2);
CMatrix(const CMatrix<M> &old); // copy ctor
~CMatrix();

int getXSize() {return s1;}
int getYSize() {return s2;}
CMatrix<M>& operator=(const CMatrix<M> &cm);// Asgnmnt constructor
CMatrix<M>& operator*(CMatrix<M> &cm);

inline M& operator()(const int i) {
    ASSERT1(i<s1 && i>=0);
    printf("CMatrix::operator(): i=%d, s1=%d\n", i, this->s1);
    return m[i];
}

inline M& operator()(const int i, const int j) {
    ASSERT2(i<s1 && j<s2);
    ASSERT2(i>=0 && j>=0);
    return m[i*s2+j];
}

int s1, s2; // dimensions of array
M *m;   // pointer to first element of matrix.
int dimensions;

The declaration:

int *oldRow=NULL;
CMatrix<int> *useRow=NULL;

And here's how they are defined:

oldRow = new int(nNodes);
useRow = new CMatrix<int>(nNodes);

I have a loop to initialize them:

printf(": &oldRow[0]=%u\n", oldRow);
printf(": &oldRow[7]=%u\n", &oldRow[7]);
printf(": &(useRow->s1)=%u\n", &(useRow->s1));
printf(": &(useRow->m)=%u\n", &(useRow->m));

for (int i=0; i<nNodes; i++) {
   *(oldRow+i)=99;
   printf("A: s1=%d\n",(*useRow).s1);
   printf("B: i=%d\n", i);
   (*useRow)(i)=0;
   printf("C: i=%d\n", i);

   for (int j=0; j<nNodes; j++) nonZeroCount(i,j)=0;
}

Finally, here's the output. Look at s1 getting overwritten:

: &oldRow[0]=39846784
: &oldRow[7]=39846812
: &(useRow->s1)=39846800
: &(useRow->m)=39846808
A: s1=8
B: i=0
CMatrix::operator(): i=0, s1=8
C: i=0
A: s1=8
B: i=1
CMatrix::operator(): i=1, s1=8
C: i=1
A: s1=8
B: i=2
CMatrix::operator(): i=2, s1=8
C: i=2
A: s1=8
B: i=3
CMatrix::operator(): i=3, s1=8
C: i=3
A: s1=99
B: i=4
CMatrix::operator(): i=4, s1=99

I know this is a bit complicated; I tried to make it as simple as possible but still keep all the relevant details in it. I was using a second plain old int array for useRow but I was getting random crashes so I thought I'd use my CMatrix class instead.

Anyway, note the addresses at the top of the output- useRow->s1 and useRow->m are put right in the middle of the oldRow array!

I know I must be doing something wrong but I don't know what it is. I also was using std:: class for both of them but was getting random crashes with those as well, I though int *oldRow might be less error-prone...

Upvotes: 0

Views: 1885

Answers (1)

Daniel Jour
Daniel Jour

Reputation: 16156

oldRow = new int(nNodes);

From that I'd suspect that the type of oldRow is a pointer to int. That above line sets this pointer to point to a single integer, and that is initialised to the value of nNodes.

You probably meant something like

oldRow = new int[nNodes]{};
//              ^      ^  \
//               array   C++11 default init of the elements

As it is, when later you're accessing "elements" of oldRow, you're invoking undefined behaviour: By writing outside of the bounds of that one element "array", you overwrite other stuff in the heap, corrupting your heap and modifying other allocated data (in this case the member fields of that instance if your class).

I also was using std:: class for both of them but was getting random crashes with those as well, I though int *oldRow might be less error-prone...

In general, unless really needed, you shouldn't use raw allocated arrays as above. The std::vector class provides a better means for runtime sized arrays. If it crashes randomly, then this is a direct sign that you're doing something wrong.

Upvotes: 1

Related Questions