Search code examples
c++pointersvectorforward-declaration

A vector member is reset and unaccessible


I have two projects, one basic client and a dynamic library. Here's what happens in the client:

int main()
{
    Scrutinizer scru;
    scru.Scrutinize();
    return 0;
}

In the DLL, The Scrutinizer class is as such (__declspec(dllexport) and such omitted for Clarity)

Header

class ProcessesGenerator;

    class Scrutinizer
    {
    public:
    Scrutinizer();
    ~Scrutinizer();
    ProcessesGenerator *ProcGenerator
    void Scrutinize();
};

The forward declaration of ProcessesGenerator was 'mandatory' for me to avoid some kind of circular reference.

Constructor in .cpp file

Here is how I initialize it:

Scrutinizer::Scrutinizer()
{
    ProcGenerator = &ProcessesGenerator();
}

More about this ProcessesGenerator class:

Header

class ProcessesGenerator
{

public:
    ProcessesGenerator();
    ~ProcessesGenerator();
    WinFinder winFinder;
    std::vector<std::string> fooCollec;

    void GenerateProcesses();
};

ProcessesGenerator.cpp

Constructor:

ProcessesGenerator::ProcessesGenerator()
{
    //winFinder = WinFinder();//problem will be the same with or without this line
    fooCollec = std::vector<std::string>{"one", "two", "three"};
}

A breakpoint in the constructor shows that the vector is initialized with the chosen values.

Problematic function:

void ProcessesGenerator::GenerateProcesses() {
    std::string foo = "bar";
    fooCollec = std::vector<std::string>{};//read access violation
    fooCollec.push_back(foo);//read access violation
    winFinder.SomeVector= std::vector<std::string>{};//read access violation
}

Once there, I Can see that the size of vector is reset to 0. Any attempt to re-initialize it, or to push an element results in read access violation .Same with the vecotr member of its WinFinder member. I guess the flaw is obvious, but I really don't get it,

Thanks!


Solution

  • Your problem is with

    Scrutinizer::Scrutinizer()
    {
        ProcGenerator = &ProcessesGenerator();
    }
    

    What you are doing is taking the address of a temporary object. That object will be destroyed and the end of that line and you will be left with a pointer that doesn't point to a valid object.

    The old way to fix it would be to use

    Scrutinizer::Scrutinizer()
    {
        ProcGenerator = new ProcessesGenerator();
    }
    

    But now you have to implement the copy constructor, copy assignment operator, and the destructor. Since you have a modern compiler what you can do instead is make ProcGenerator a std:unique_ptr<ProcessesGenerator> and then Scrutinizer() becomes

    Scrutinizer::Scrutinizer() : ProcGenerator(make_unique<ProcessesGenerator>()) {}
    

    I would also like to add that &ProcessesGenerator(); should not even compile. Unfortunately MSVS has a non-standard extension that allows this to compile. You can turn on the /Za compiler option (enforce ANSI compatibility) and then you should get an error like

    error C2102: '&' requires l-value