-5

I'm trying to make this code as generic as possible to help future wanderers.

A.h

class A {
    protected:
        A* pointToAnother;
    public:
        A();
        func();
}

A.cpp

A::A() {
    pointToAnother = nullptr;
}

A::func() {
    // does something
}

B.h

#include "A.cpp" //Using A.h gave me linking errors

class B {
    protected:
        A* pointToA;
    public:
        B();
        do_something();
}

B.cpp

B::B() {
    A tmp;
    pointToA = &tmp;
}

B::do_something() {
    pointToA->func();
}

int main() {
    B test;
    B.do_something();
}

When do_something() is called, before pointToAnother->func() runs, pointToA->pointToAnother correctly points to 0x00000000. However, once func() runs, pointToAnother is now pointing at 0xcccccccc BEFORE running what's inside of func(), which breaks my program.

I tried implementing the rule of three, thinking the bug had something to do with destruction, but that had no dice. I ran into a similar problem in another program (modifying an object in a vector by calling a function of said object, only for the previous position in the vector to now be pointing to 0xcccccccc) that I gave up on.

Thanks

Master Bob
  • 63
  • 1
  • 6
  • 4
    `pointToA->func();` You have undefined behavior. It might behave different on other platforms or when code is changed. So what? – πάντα ῥεῖ Jun 10 '17 at 06:11
  • 4
    `A tmp;` - this object dies as soon as the constructor of B returns. – Mat Jun 10 '17 at 06:14
  • 3
    How did you run this code? It doesn't compile! – Support Ukraine Jun 10 '17 at 06:17
  • 1
    As for your edit: That's how MSVC implements this in debug versions of your build to make it easier to find these kind of bugs. Still it may differ for release builds, or what other compilers do. – πάντα ῥεῖ Jun 10 '17 at 06:25
  • 1
    "Using A.h gave me linking errors" that's because you need to add A.cpp to the list of files being compiled. Including the source file as you're doing it is a bad idea. – Daniel Jour Jun 10 '17 at 06:26

1 Answers1

1

You have:

B::B() {
    A tmp;
    pointToA = &tmp;
}

In the function, tmp is a function local variable. It gets deleted when the function returns and you are left with a dangling pointer value in pointToA. If you dereference the pointer later, you invoke undefined behavior.

You can resolve that problem by couple of means.

  1. Change the variable pointToA to be objectA, i.e. change it from being a pointer to A to be an object of type A.

    class B {
        protected:
            A objectA;
    
  2. Allocate memory using new A and assign it to pointToA.

    B::B() {
        pointToA = new A;
    }
    

If you use the second method, make sure to read up on The Rule of Three and update your class accordingly.

R Sahu
  • 204,454
  • 14
  • 159
  • 270