1

I'm running into some strange problems when assigning the reference of a pointer to a variable: the local code works correctly, but it causes memory access errors elsewhere:

//This works fine
Gridcell* g = model.gc;
cout << g->LC_updated << " " << g << endl;

//When I include this, the program crashes elsewhere
//But output from this line is OK
Gridcell gc = *g;
cout << "Var:" << gc.LC_updated << &gc << endl;

(Gridcell is a class, which doesn't have a no-args constructor) I'm not a C++ expert, and this is a fairly large external library, but I can't understand why an assignment to a locally scoped variable should cause problems elsewhere. Can anyone shed some light on this?

mo-seph
  • 6,073
  • 9
  • 34
  • 35

5 Answers5

3

You don't give enough information to solve the problem. There is nothing inherently wrong with the code you've posted.

My guess is that Gridcell lacks a proper copy constructor and so when gc goes out of scope it deletes things that *g is still referring to.

This line of code:

Gridcell gc = *g;

is where the copy constructor is being invoked. It's essentially equivalent to saying this:

Gridcell gc(*g);

which invokes the Gridcell::Gridcell(const Gridcell &) constructor, otherwise known as the copy constructor. The copy constructor is special in that if you don't have one, the compiler will automatically generate one for you. The automatically generated one typically just invokes the copy constructor of each individual member variable, including the pointers. For basic types, like int or Foo *, the copy constructor simply makes an exact copy.

For example, if you have code like this:

class Foo {
 public:

   Foo() : msg_(new char[30]) { strcpy(msg_, "I'm Foo!"); }
   ~Foo() { delete [] msg_; }

 private:
   char *msg_;
};

void aFunction(Foo *aFoo)
{
   Foo myfoo = *aFoo;
}

void anotherFunction()
{
   Foo localfoo;
   aFunction(&localfoo);
}

It will crash. localfoo will allocate character array. The line Foo myfoo = *aFoo will call the copy constructor which will make a straight copy of the pointer. Then the destructor for myfoo will be called and the memory will be freed. Then the destructor for localfoo will be called and the memory will be freed again, resulting in a crash on many systems.

Omnifarious
  • 54,333
  • 19
  • 131
  • 194
  • By default, the copy constructor does a shallow copy (i.e. pointer values are copied). So, the destructor, if it deletes members, deletes the memory used by the original object as well, so the pointer in the original object is pointing at freed memory. – Skizz Mar 16 '11 at 11:18
  • Thanks both Omnifarious and Skizz, that really helped! – mo-seph Mar 16 '11 at 11:25
1

You need to make sure that Gridcell has a proper copy constructor. This is only required if Gridcell manually manages resources, which almost never should be the case. If it the the case for you, you will need The Big Three.

If you need more specific help, post the class definition of Gridcell, along with constructors and destructor, if you have them.

If your intention is not to copy, you can use a reference:

Gridcell & gc = *g;
cout << "Var:" << gc.LC_updated << &gc << endl;

The usage is the same, but it will not create copy.

Community
  • 1
  • 1
Björn Pollex
  • 75,346
  • 28
  • 201
  • 283
0

Is model.gc a local variable?

If so, when it goes out of scope it ceases to exist. And any pointers to it are no longer valid.

Steve Wellens
  • 20,506
  • 2
  • 28
  • 69
0

@Space_C0wb0y is right.

Imagine that you have attributes in your Gridcell class that are pointers (arrays or objects that are explicitly allocated through new).

When you assgin an object of type Gridcell from another one, the default copy constructor makes a shallow copy. It copies the value of those pointers, but does not create new objects/arrays for the new attributes of yout new object.

If you split

cout << "Var:" << gc.LC_updated << &gc << endl;

in two lines:

cout << "&gc" <<&gc << endl;
cout << "Var:" << gc.LC_updated << endl;

You will probably see that the fault goes to the line where your reference LC_updated (I don't know what is it).

Check your constructors to see how it is being initialized.

j4x
  • 3,595
  • 3
  • 33
  • 64
  • Unfortunately, that doesn't help - the fault doesn't happen here. It's because the destructor gets called on gc when it goes out of scope, and that destroys the object pointed to by g, which is used elsewhere. – mo-seph Mar 16 '11 at 11:33
0

This line:

GridCell gc = *g;

Does a copy of the instance pointed by g. If the GridCell does not define a copy-constructor (a constructor of signature GridCell(const GridCell& other)), then a default one is provided by the compiler that does a copy of each member. For member that are of a pointer type, this is just a copy of the pointer, and thus both gc and the instance pointed by g will point to the same memory.

When the gc variable goes out of scope, its destructor is called. If the GridCell class does manage some memory, does not provide a copy-constructor (or it is incorrect), and release the memory in its destructor, then the GridCell instance pointed by g will point to released memory after this point.

Generally, when a class manage some memory, it must override the destructor, the copy-constructor and the assignment operator. This is the rule of three.

Please note that the assignment will also perform slicing if g point to a subclass of GridCell and that can also bring other problems. If you want to use the gc variable as an alias not to have to use *g everywhere, you should consider using a reference (or a const-reference) instead of making a copy (especially if the object does manager lots of memory, copy can be expensive).

Sylvain Defresne
  • 42,429
  • 12
  • 75
  • 85