Why is this method returning -858993460

Question

The printResults() method below (called from the main method at the bottom) returns -858993460 for all four values. Why is this? I have confirmed with cout statements that the numbers and calculations are correct within the doCalc() method, so I'm assuming the error is in the way I'm using pointers and calling the printResults() method...

typedef int matrix[2][2] ;

struct matrices {
matrix a;
matrix b;
};

...getInput() method constructs 

matrix* doCalc (matrices m){
matrix toReturn;
char input;
cout << "Which calculation would you like to perform - (M)ultiply, (A)dd, (S)ubtract?";
cin >> input;
switch(input){
case 'M':
    toReturn[0][0] = ((m.a[0][0])*(m.b[0][0]));
    cout << "XX " << ((m.a[0][0])*(m.b[0][0]));
    toReturn[0][1] = (m.a[0][1]*m.b[0][1]);
    cout << "YY " <<  (m.a[0][1]*m.b[0][1]);
    toReturn[1][0] = (m.a[1][0]*m.b[1][0]);
    toReturn[1][1] = (m.a[1][1]*m.b[1][1]);
    break;
case 'A':
    toReturn[0][0] = (m.a[0][0]+m.b[0][0]);
    toReturn[0][1] = (m.a[0][1]+m.b[0][1]);
    toReturn[1][0] = (m.a[1][0]+m.b[1][0]);
    toReturn[1][1] = (m.a[1][1]+m.b[1][1]);
    break;
case 'S':
    toReturn[0][0] = (m.a[0][0]-m.b[0][0]);
    toReturn[0][1] = (m.a[0][1]-m.b[0][1]);
    toReturn[1][0] = (m.a[1][0]-m.b[1][0]);
    toReturn[1][1] = (m.a[1][1]-m.b[1][1]);
    break;
}
return &toReturn;

}

void printResult(matrix m){
cout<<"---RESULT---\n";
cout << m[0][0] << "  " << m[0][1] << "\n";
cout << m[1][0] << "  " << m[1][1] << "\n";

}

void main() {
matrices m = getInput();
cout << m.a[0][0] << "  " << m.a[0][1] << "\n";
cout << m.a[1][0] << "  " << m.a[1][1] << "\n\n";
cout << m.b[0][0] << "  " << m.b[0][1] << "\n";
cout << m.b[1][0] << "  " << m.b[1][1] << "\n";

matrix* calc = doCalc(m);
matrix c = &calc;

printResult(*calc);

}

You're returning a pointer to a local variable. Once the function returns that variable no longer exists. — Captain Obvlious, May 07 '13 at 01:19

smitec · Accepted Answer · 2013-05-07T01:26:20.680

2

the matrix toReturn will be destroyed when the function exits, you will need to use some form of persistent memory allocation (look up new and delete). As such all the values are just nonsense.

edited May 07 '13 at 01:26

answered May 07 '13 at 01:21

smitec

3,049
1
16
12

It should be new in C++ not malloc. – Borgleader May 07 '13 at 01:22
Look up `new` and `delete` if you must or maybe return by value instead. – Captain Obvlious May 07 '13 at 01:22
By value is best here. In second place comes a smart pointer, as it's clear to the caller that they now own the memory, and it will be freed regardless of whether they do so manually or not. – chris May 07 '13 at 01:23
@smitec I would call malloc inside the doCalc() method, or in the main method? – May 07 '13 at 01:23
@CaptainObvlious But how can I return an array by value in C? – May 07 '13 at 01:24
@user2356560 `std::vector` works pretty well for C++. For C I would retag the question. – Captain Obvlious May 07 '13 at 01:25
@Borgleader oh yes sorry I was thinking C by default. – smitec May 07 '13 at 01:25
Here's your rep back :) – Borgleader May 07 '13 at 01:27

score 0 · Answer 2 · answered May 07 '13 at 01:23

0

matrix toReturn; is a local variable, allocated on the stack. After doCalc returns, its contents are undefined. In this case, the memory at its old address gets overwritten with random garbage.

answered May 07 '13 at 01:23

1''

26,823
32
143
200

score 0 · Answer 3 · answered May 07 '13 at 01:27

0

toReturn will be destroyed with the stack frame when doCalc is finished.

Instead of returning the pointer pass it to the function you are calling.

void doCalc (matrices m, matrix* pReturnMatrix)

matrix* calc = new matrix();
doCalc(m, calc);

answered May 07 '13 at 01:27

james82345

530
4
13

score 0 · Answer 4 · answered May 07 '13 at 01:29

toReturn is a local variable in the doCalc function. It will no longer exist when the function returns.

An easy way to fix this is to pass a reference to the matrix that will store the result:

matrix* doCalc (matrices m, matrix& toReturn ){

Then remove this line from doCalc:

matrix toReturn;

And change how the function is called from this:

matrix* calc = doCalc(m);
matrix* calc = doCalc(m);
matrix c = &calc;

printResult(*calc);

To this:

matrix calc;
doCalc(m,calc);
printResult(calc);

There are other ways to fix this, e.g. new/delete as suggested in other places. However, now we will go into who allocates and who owns the memory. It won't be clear from the function signature alone who is responsible for what.

score 0 · Answer 5 · answered May 07 '13 at 01:33

You can avoid managing dynamically allocated memory and requiring the caller supply an lvalue if you return by value.

matrix doCalc (const matrices& m)
{
    matrix toReturn;

    // Code here

    return toReturn;
}

You can assign the results to a matrix object or use it in an expression like below.

if(doCalc(m).a[0][0] == 1)
{
     // do something
}

or

matrix mat(doCalc(m));
if(mat.a[0][0] == 1)
{
     // do something
}

Why is this method returning -858993460

5 Answers5