0

I wrote this simple function to learn about arrays in C++ (I am using CERN's ROOT as a C++ compiler):

int** vec() {
  int n = 2;
  int m = 3;

  int *pointer[m];                                                                                                                              

  for (int i=0; i<m; i++) {
    pointer[i] = new int[n];
  }

  for (int i=0; i<m; i++) {
    for (int j=0; j<n; j++) {
      pointer[i][j] = -1;
    }
  }

  return pointer;
}

After compiling the code, which I have saved as test.C, I create two pointers to two-arrays in what seems to me an identical manner, but with different results. The first was works exactly as I expected both methods to work:

root [0] .L test.C+
root [1] 
root [1] int** mat1;
root [2] int** mat2;
root [3] mat1 = vec();
root [4] mat2 = vec();
root [5] mat1[0][0]
(int)(-1)
root [6] mat2[0][0]
(int)(-1)

Okay, nothing surprising there. But if I do all the stuff with mat1 before getting to mat2, I get the following very different result:

root [0] .L test.C+
root [1] 
root [1] int** mat1;
root [2] mat1 = vec();
root [3] mat1[0][0]
(int)(-460363344)
root [4] 
root [4] int** mat2;
root [5] mat2 = vec();
root [6] mat2[0][0]
(int)(-1)

In this case, it appears that my function has not worked as far as mat1 is concerned. Can anyone help me understand what is going on here?

meatball
  • 76
  • 5
  • You're returning a local variable address (`pointer`), which invokes undefined behavior upon reference on the caller-side. The impending memory leaks are pure bonus points. – WhozCraig Jun 27 '14 at 00:35
  • 1
    Do you see any **warnings** when you compile this code? – Drew Dormann Jun 27 '14 at 00:36
  • The local variable is destroyed when the function exits and is no longer available (undefined behavior). Try using a static array or a dynamically allocated array. – Thomas Matthews Jun 27 '14 at 00:36
  • http://stackoverflow.com/questions/12573816/what-is-an-undefined-reference-unresolved-external-symbol-error-and-how-do-i-fix – chris Jun 27 '14 at 00:37
  • @DrewDormann No warnings – meatball Jun 27 '14 at 00:39
  • @user3757134 then the real problem is that you are compiling with warnings disabled. :) I'm not familiar with CERN'S ROOT, but I'm wary of a compiler that compiles that without warning you that your code returns a pointer to a local array. – Drew Dormann Jun 27 '14 at 00:40
  • This code cannot be possibly compiled as a compliant C++ code. C++ language does not support array declarations with sizes represented by non-constants (like `int *pointer[m]`) in your case. A feature like that exists in C, but not in C++. Even if your compiler accepts it (as a non-standard extension), the problem, as others have already noted, is that you are returning a pointer to a local array. – AnT stands with Russia Jun 27 '14 at 00:40
  • @AndreyT but g++ *does* support it via non-standard extension (unfortunately). – WhozCraig Jun 27 '14 at 00:41
  • @AndreyT Why does declaring and calling my pointers in different orders change the behavior? I would expect both orders to behave identically, problems with my pointer declaration or not. – meatball Jun 27 '14 at 00:46
  • @user3757134: You are reading invalid memory, which is generally filled with random garbage. One thing about random garbage is that it is *random*. It can change for any reason or without a reason. There's nothing unusual in the fact that "calling your pointers in different orders" changes that random garbage (whatever you mean by "calling pointers"). – AnT stands with Russia Jun 27 '14 at 02:54

1 Answers1

1

I see the following problems:

Problem 1

int *pointer[m];

is not legal C++ code.

Problem 2

When you execute

return pointer;

you are returning a pointer to memory that is not valid after the function returns.

The simple fix is to replace the line

int *pointer[m];

by

int ** pointer =  new int*[m];
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • But now you've got a new problem... deleting that memory (pun intended). You should probably use a vector instead. – Cogwheel Jun 27 '14 at 00:46
  • @Cogwheel I would, but a vector of vectors doesn't work in ROOT – meatball Jun 27 '14 at 00:48
  • A vector of pointers should work. `&vec[0]` would be equivalent to the int** you're returning now. Possibly even a vector of unique_ptrs, and then you wouldn't even have to manually manage the elements' deletion. – Cogwheel Jun 27 '14 at 01:10
  • Thanks, I might do that. For my purposes, I am only doing int** pointer = vec() inside of a for loop, which I think will take care of memory things, but I don't really know. – meatball Jun 27 '14 at 01:13