1

Here's my code:

#include "stdafx.h"
#include "math.h"
#include <iostream>

using namespace std;

double Calc_H(double Q, double Head, double *constants)
{
    return (constants[0] * pow(Q, 4) + constants[1] * pow(Q, 3) + constants[2] * pow(Q, 2) + constants[3] * Q + constants[4] - Head);
}

double Calc_dH(double Q, double *constants)
{
    return (4 * constants[0] * pow(Q, 3) + 3 * constants[1] * pow(Q, 2) + 2 * constants[2] * Q + constants[3]);
}

double NewtonRaphson(double Head, double first_guess, double max_error, double * constants)
{
    double Q_iter = first_guess;
    int iter_counter = 1;
    cout << constants << endl << constants[0] << endl << constants[1] << endl;
    while (abs(Calc_H(Q_iter, Head, constants)) > max_error || iter_counter > 1000)
    {
        Q_iter = Q_iter - Calc_H(Q_iter, Head, constants) / Calc_dH(Q_iter, constants);
        iter_counter++;
    }
    return Q_iter;
}

double * Calc_constants(double freq)
{
    double * pointer;
    double constants[6];
    constants[0] = -1.2363 + 2.3490 / 10 * freq - 1.3754 / 100 * pow(freq, 2) + 2.9027 / 10000 * pow(freq, 3) - 2.0004 / 1000000 * pow(freq, 4);
    constants[1] = 1.9547 - 4.5413 / 10 * freq + 3.5392 / 100 * pow(freq, 2) - 8.1716 / 10000 * pow(freq, 3) + 5.9227 / 1000000 * pow(freq, 4);
    constants[2] = -5.3522 - 4.5413 / 10 * freq - 1.3311 / 100 * pow(freq, 2) + 4.8787 / 10000 * pow(freq, 3) - 4.8767 / 1000000 * pow(freq, 4);
    constants[3] =  3.8894 / 100 + 3.5888 / 10 * freq + 1.0024 / 100 * pow(freq, 2) - 5.6565 / 10000 * pow(freq, 3) + 7.5172 / 1000000 * pow(freq, 4);
    constants[4] = -8.1649 + 5.4525 / 10 * freq - 3.2415 / 100 * pow(freq, 2) + 8.9033 / 10000 * pow(freq, 3) - 9.0927 / 1000000 * pow(freq, 4);
    constants[5] =  2.1180 / 10 + 5.0018 / 100 * freq + 6.0490 / 1000 * pow(freq, 2) - 1.5707 / 100000 * pow(freq, 3) + 3.7572 / 10000000 * pow(freq, 4);

    pointer = constants;
    return pointer;
}

int _tmain(int argc, _TCHAR* argv[])
{

    double * constants;
    //Determine constants based on freq (see manual pump)
    double freq;
    cin >> freq; 
    double head;
    cin >> head;
    constants = Calc_constants(freq);
    cout << constants[0] << endl << constants[1] << endl << constants << endl;
    cout << NewtonRaphson(head, 0, 0.001, constants) << endl;
    cin >> freq;    
    return 0;
}

The function Calc_constants returns a pointer to an array of calculated values. So far so good.

The function NewtonRaphson takes the pointer to this array as a parameter. When dereferencing this pointer in this function it returns different results for constants[0] and constants[1]. I find this very strange, because the address the pointer is 'pointing' to is the same.

To clarify this is the output (cout):

-0.09505
2.6008
OOD6F604
00D6F604
-9.25596e+0.61
-9.25596e+0.61
-1.08038e-0.62
Bernhard Barker
  • 54,589
  • 14
  • 104
  • 138
  • possible duplicate of [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – Raymond Chen Jul 24 '15 at 14:52

2 Answers2

2
double * Calc_constants(double freq)
{
    double * pointer;
    double constants[6];

Calc_constants allocates memory for this array on its stack, not on heap. When this function returns, this block of memory may be allocated for some other purpose, hence is not supposed to be accessed outside of this function. Because of this, when pointer is returned, and used later, it leads to unpredictable results.

constants array needs to be allocated either in main or on heap so its lifetime is long enough for this kind of usage.

In this while loop condition,

while (abs(Calc_H(Q_iter, Head, constants)) > max_error || iter_counter > 1000)

I guess, it should be iter_counter < 1000.

ramana_k
  • 1,933
  • 2
  • 10
  • 14
0

There's not much C++ in your code. First of all, let's remove the non-standard stuff:

#include "stdafx.h"
#include "math.h"
#include <iostream>

Should become:

#include <math.h>
#include <iostream>

int _tmain(int argc, _TCHAR* argv[])

Should become:

int main()

Then there are C-style arrays all over the place. You don't want to do that. Use std::array or std::vector and the problem will disappear all by itself.

Here is an example with std::vector:

#include <math.h>
#include <iostream>
#include <vector>

double Calc_H(double Q, double Head, std::vector<double> const& constants)
{
    return (constants[0] * pow(Q, 4) + constants[1] * pow(Q, 3) + constants[2] * pow(Q, 2) + constants[3] * Q + constants[4] - Head);
}

double Calc_dH(double Q, std::vector<double> const& constants)
{
    return (4 * constants[0] * pow(Q, 3) + 3 * constants[1] * pow(Q, 2) + 2 * constants[2] * Q + constants[3]);
}

double NewtonRaphson(double Head, double first_guess, double max_error, std::vector<double> const& constants)
{
    double Q_iter = first_guess;
    int iter_counter = 1;
    std::cout << constants.data() << std::endl << constants[0] << std::endl << constants[1] << std::endl;
    while (abs(Calc_H(Q_iter, Head, constants)) > max_error && iter_counter < 1000)
    {
        Q_iter = Q_iter - Calc_H(Q_iter, Head, constants) / Calc_dH(Q_iter, constants);
        iter_counter++;
    }
    return Q_iter;
}

std::vector<double> Calc_constants(double freq)
{
    std::vector<double> constants(6);
    constants[0] = -1.2363 + 2.3490 / 10 * freq - 1.3754 / 100 * pow(freq, 2) + 2.9027 / 10000 * pow(freq, 3) - 2.0004 / 1000000 * pow(freq, 4);
    constants[1] = 1.9547 - 4.5413 / 10 * freq + 3.5392 / 100 * pow(freq, 2) - 8.1716 / 10000 * pow(freq, 3) + 5.9227 / 1000000 * pow(freq, 4);
    constants[2] = -5.3522 - 4.5413 / 10 * freq - 1.3311 / 100 * pow(freq, 2) + 4.8787 / 10000 * pow(freq, 3) - 4.8767 / 1000000 * pow(freq, 4);
    constants[3] =  3.8894 / 100 + 3.5888 / 10 * freq + 1.0024 / 100 * pow(freq, 2) - 5.6565 / 10000 * pow(freq, 3) + 7.5172 / 1000000 * pow(freq, 4);
    constants[4] = -8.1649 + 5.4525 / 10 * freq - 3.2415 / 100 * pow(freq, 2) + 8.9033 / 10000 * pow(freq, 3) - 9.0927 / 1000000 * pow(freq, 4);
    constants[5] =  2.1180 / 10 + 5.0018 / 100 * freq + 6.0490 / 1000 * pow(freq, 2) - 1.5707 / 100000 * pow(freq, 3) + 3.7572 / 10000000 * pow(freq, 4);

    return constants;
}

int main()
{
    //Determine constants based on freq (see manual pump)
    double freq;
    std::cin >> freq; 
    double head;
    std::cin >> head;
    std::vector<double> constants = Calc_constants(freq);
    std::cout << constants[0] << std::endl << constants[1] << std::endl << constants.data() << std::endl;
    std::cout << NewtonRaphson(head, 0, 0.001, constants) << std::endl;
    std::cin >> freq;    
    return 0;
}

(I've also modified the while loop to what I guess is what you intended.)

As you can see, element access has the same syntax as C arrays. A pointer to the data encapsulated by the std::vector is obtained with data(). I've added this since your original code printed the address of the array; you will rarely need data() in your real code for this kind of application.


Now, as far as your original code is concerned:

double * Calc_constants(double freq)
{
    double * pointer;
    double constants[6];
    // ...    
    pointer = constants;
    return pointer;
}

This simply produces undefined behaviour. constants is a local variable. The six elements you create here are destroyed when the function returns, yet you keep a pointer to them. The C++ language makes no guarantees as to what will happen, should you try to dereference that pointer later on (as you do). With some luck, the program would have crashed immediately, showing you that there is a serious error, rather than generating nonsense output.

You were a bit unlucky as well not to get a compiler warning for this. Had you not used the redundant pointer variable, then you might have received a warning (at least with VC 2013).

Simple example:

double * Calc_constants()
{
    double constants[6];
    return constants;
}

int main()
{
    double* ptr = Calc_constants();
}

VC 2013 warning:

warning C4172: returning address of local variable or temporary

With std::vector, the data is allocated internally such that you can safely return objects. You can use standard container objects as safely as simple ints, without raw-pointer complexities sprinkled all over your code.

Christian Hackl
  • 27,051
  • 3
  • 32
  • 62