c++ pointers and variable scope

Question

I am trying to create a tree structure using some handler functions that are called while reading a stream. I think the problem is that my variables are created in the function's scope and disappear when the function ends, leaving pointers that point to nothing.

I am not sure what approach to take to keep the objects in memory, whilst still allowing the tree to be scalable.

I have made a simplified version of the code: it compiles and runs but the parent-child relationships of the 'Segment' objects are all wrong.

class Segment
{
public:
    Segment* parent;
    list<Segment*> children;
    string name;
};

void OpenSegment(Segment* p_segCurrentseg);
void CloseSegment(Segment* p_segCurrentseg);

int _tmain(int argc, _TCHAR* argv[])
{
    Segment parent;
    parent.name="parent";
    Segment* p_segCurrentseg=&parent;
    OpenSegment(p_segCurrentseg);
    OpenSegment(p_segCurrentseg);
    OpenSegment(p_segCurrentseg);
    CloseSegment(p_segCurrentseg);
    return 0;
}

void OpenSegment(Segment* p_segCurrentseg)
{
    Segment child;
    child.name="child";
    p_segCurrentseg->children.push_front(&child);
    child.parent=p_segCurrentseg;
    p_segCurrentseg=&child;
}

void CloseSegment(Segment* p_segCurrentseg)
{
    p_segCurrentseg=p_segCurrentseg->parent;
}

Maybe it compiles for you but it certainly doesn't compile for me. At any rate, it looks like you need to use new. — ooga
– ooga, Commented May 19, 2014 at 5:14

R Sahu · Accepted Answer · 2014-05-19 05:15:01Z

2

There are couple of problems in your code.

You are passing p_segCurrentseg by value and assigning to another pointer. This has no effect on the variable in the calling function.
As you already suspected, you are trying to assign p_segCurrentseg to point to a variable that will be gone when you return from the function.

What you can do:

Pass p_segCurrentseg by reference to a pointer.
Create an object from the heap and assign p_segCurrentseg to point to it.

Here's my suggestion for OpenSegment:

void OpenSegment(Segment*& p_segCurrentseg)
{
    Segment* child = new Segment;
    child->name="child";
    p_segCurrentseg->children.push_front(child);
    child->parent=p_segCurrentseg;
    p_segCurrentseg=child;
}

answered May 19, 2014 at 5:15

R Sahu

207k15 gold badges168 silver badges291 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

user3651203 Over a year ago

Thanks so much! you are legend

Kevin Le - Khnle · Accepted Answer · 2014-05-19 05:23:11Z

The problem is in the OpenSegment() method, particularly in these 3 lines:

Segment child;
child.name="child";
p_segCurrentseg->children.push_front(&child);

First, child is a local variable and created on the stack. You then push the address of child into your list. When OpenSegment() returns, the address of child contains garbage since storage for local variables are deallocated.

The solution is to define child as a pointer to Segment, create it on the heap so it lives even after OpenSegment() returns. You have to make sure to deallocate its memory too. The proper place is to define a destructor for your Segment class. In it, iterate through the list (of children segments) and deallocate the memory for each child.

Collectives™ on Stack Overflow

c++ pointers and variable scope

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related