1

I am computing properties of boundary layer flow through a duct. I have a class CChannel which stores geometry of the duct, CFlow which holds global properties of the fluid and CNode which stores local parameters of the boundary layer. When I execute the program in the current form the first element of the GridPoints vector (variable "alpha") inside CChannel is assigned the same memory location as Uinf which is a private member of the CFlow class. When I change the latter class so that the fields it holds are no longer pointers but regular variables the problem disappears. I also tried reserving memory space for the GridPoints vector inside the class constructor but without any effect. When I was searching for the answer I found that this may have been caused by the in-built code optimiser but didn't manage to learn anything else. (If this is so, how can I get around this without losing efficiency?)I am guessing the problem arises because of the differences between two different memory allocation modes (heap vs stack). I would still like to find out why is this happening exactly so I can store the global flow parameters as pointers and avoid this problem in the future.

Program.cpp

#include <iostream>

#include "Channel.h"    // stores the channel geometry
#include "Flow.h"   // stores the fluid properties and free stream data
#include "Node.h"       // holds the local BL flow properties, e.g. BL thickness, lambda, etc.

using namespace std;

int main(void)
{
    int NoNodes=21;
    CChannel MyChan(4, 1.2, .8);    // L, h1, h2
    MyChan.MeshUniform(NoNodes);

    CFlow Flow1(.5,1.529e-5,1.19);  // Uinf, niu, ro

    for (int i=0;i<NoNodes;i++)
    {
        MyChan.GridPoints->at(i).GetAlpha();
    }
    return(0);
}

Node.h

#pragma once
class CNode
{
public:
    double *alpha, *x, *lambda; // properties dependent on the Pollhausen velocity profile

    CNode(void);
    ~CNode(void);

    void GetAlpha(void);    // calculates alpha

};

Node.cpp

#include "Node.h"
#include <iostream>

CNode::CNode(void)
{
    alpha=new double;

    lambda=new double;
    *lambda=0;
}

CNode::~CNode(void)
{
    delete alpha, x, lambda;
}

void CNode::GetAlpha(void)
{
    *alpha=(.3-*lambda/120.);
}

Flow.h

#pragma once
class CFlow
{
private:
    double *Uinf, *niu, *ro;
public:
    CFlow(double, double, double);
    ~CFlow(void);
};

Flow.cpp

#include "Flow.h"

CFlow::CFlow(double u, double visc, double den)
{
    Uinf=new double;
    niu=new double;
    ro=new double;
    *Uinf=u;    // free stream velocity (assumes the inflow is parallel to the channel's CL) [m/s]
    *niu=visc;  // kinematic viscosity of the fluid [m^2/s]
    *ro=den;    // density of the fluid [kg/m^3]
}


CFlow::~CFlow(void)
{}

Channel.h

#pragma once
#include <vector>

#include "Node.h"

class CChannel
{
public:
    double *L, *h1, *h2;    // h1 & h2 defined from the CL => make use of the problem assumed to be symmetric
    std::vector<CNode> *GridPoints; // stores data for each individual grid point

    CChannel(double, double, double);
    ~CChannel(void);

    void MeshUniform(int);  // creates a uniform distribution of nodes along the length of the channel
};

Channel.cpp

#include "Channel.h"

CChannel::CChannel(double length,double height1,double height2)
{
    L=new double;   // allocate memory
    h1=new double;
    h2=new double;
    GridPoints = new std::vector<CNode>;

    *L=length;      // assign input values
    *h1=height1;
    *h2=height2;
}


CChannel::~CChannel(void)
{
    delete L, h1, h2, GridPoints; // delete all the members of the class
}


void CChannel::MeshUniform(int NoNodes)
{
    GridPoints->resize(NoNodes);    // resize the vector
    double dx=*L/(NoNodes-1);   // increment of length between each pair of nodes
    for (int i=0; i<NoNodes; i++)
        *GridPoints->at(i).x=0.+i*dx;   // assign the location to each node
}
Artur
  • 445
  • 6
  • 13
  • 1
    The situation you describe does not happen in working code. If it is happening it's because your code is bugged somehow. It has nothing to do with 'memory allocation modes' or anything like that. One question, how do you know this is happening? It's definitely possible that you are simply mistaken. – john Oct 27 '12 at 19:35
  • I ran the program in debug mode in visual studio and checked that the clashing elements are allocated the same memory. And, as I said, when I changed the CFlow fields from double* to double the problem disappeared. – Artur Oct 27 '12 at 19:38
  • BTW having looked at your code you do have a lot of errors and misunderstandings in it. And you definitely don't need to allocate all that memory. I'm not seeing **anything** in your code that needs a pointer. So my advice would be to forget about this problem, whatever is causing it, and do things the easy way and don't use any pointers at all. Your program doesn't need them. – john Oct 27 '12 at 19:38
  • Unfortunately the code posted doesn't compile so I can't check for myself. – john Oct 27 '12 at 19:45
  • I simply compared the pointers to the two elements, i.e. Flow1.Uinf and MyChan.GridPoints->at(0).alpha, and saw that they are identical,if that's what you're asking for because I'm not sure what you mean exactly. – Artur Oct 27 '12 at 19:45
  • OK, well I'd like to check myself but two things are stopping me compiling your code, 'beta' and 'gamma' in Node.cpp are undeclared variables, and CNode doesn't have a member called x (in CChannel::MeshUniform). Give me the correct code for those two compiler errors and I'll check for myself. – john Oct 27 '12 at 19:48
  • Ah, I must have missed them when simplifying the code (it is actually a much bigger project). I will update the sample in a sec. – Artur Oct 27 '12 at 19:49
  • TLDR. Can we have a testcase and a summary? – Lightness Races in Orbit Oct 27 '12 at 20:13
  • It's already been answered pretty well I think, thanks. – Artur Oct 27 '12 at 20:20

2 Answers2

1

As already described you do not need all these pointers - change them to be raw variable.

If you some day come to the situation where pointers are needed then remember of Rule of Three: What is The Rule of Three?.

You broke this rule by not defining copy constructor and assignment operator in your classes, like in this particular class:

class CNode
{
public:
    double *alpha, *lambda; // properties dependent on the Pollhausen velocity profile

    CNode(void);
    ~CNode(void);

    void GetAlpha(void);    // calculates alpha

};

You are using this CNode in std::vector<CNode> - so there you are suffering from wrong copying of this CNode object.

So you need to add copy constructor and assignment operator - then your problem should disappear even if you will be still using pointers, like in this simple example class:

class Example {
public:
  Example() : p(new int()) {}
  ~Example() { delete p; }
  Example(const Example& e) p(new int(e.p?*e.p:0)) {}
  Example& operator = (Example e)
  {
      std::swap(e.p, p);
      return *this;
  }
private:
  int* p;  
};
Community
  • 1
  • 1
PiotrNycz
  • 23,099
  • 7
  • 66
  • 112
  • Thanks a lot, I understand what was wrong now. I will just get read of the pointers and keep it simple for this case and will keep 'The Rule of Three' in mind for the future. (Serio, wielkie dzieki :) ) – Artur Oct 27 '12 at 20:07
1

It's compilcated but the explanation is that in your pointer code you haven't written objects that can be copied (as Piotr says you haven't followed the rule of three). Because of this bug what is happening is that memory is begin allocated, memory is being freed and then allocated again. By coincidence when you allocate memory again it reuses the same address that was just freed. That is why you see the same pointer values.

MyChan.GridPoints->at(0).alpha is a pointer but the memory it is pointing at has been freed. Then you allocate some more memory for Flow1.Uinf and it reuses the same freed memory that MyChan.GridPoints->at(0).alpha is pointing at. So you get the same value for both pointers.

One other misunderstanding you have

delete L, h1, h2, GridPoints; // delete all the members of the class

does not delete all members of the class. Only

delete L; // delete all the members of the class
delete h1;
delete h2;
delete GridPoints;

does that. What you wrote deletes GridPoints only. You might want to look up the C++ comma operator for an explanation as to why.

john
  • 85,011
  • 4
  • 57
  • 81