Why is my pointer assignment causing a seemingly unrelated value to change?

Question

I'm trying to solve an interval problem using double pointers. The basic premise is to store double pointers to an Interval struct in my map so I can update the interval without needing to iterate over any other values in my map. Using double pointers seems to be causing some unexpected changes to my intervals, and I don't understand why.

Here's my current code:

#include <iostream>
#include <vector>
#include <utility>
#include <unordered_map>
using namespace std;

int main()
{
    vector<pair<int, char>> sequence = { 
        {0,'a'},
        {1,'-'},
        {2,'b'},
        {3,'c'},
        {4,'-'},
        {5,'d'},
        {6,'e'},
        {7,'-'},
        {8,'f'},
        {10,'g'},
        {11,'-'},
        {12, '-'}
    };

    struct Interval {
        string word;
        bool startH;
        bool endH;
        Interval(string word) : word(word), startH(false), endH(false) {}

    };

    Interval* hyphen = new Interval("-");
    Interval** hPtr = &hyphen;

    auto consolidate = [&hPtr](Interval**& left, Interval**& right) {
        //cout << "\t(" << (*left)->word << " " << (*right)->word << ")" << endl;
        if (left == hPtr && right == hPtr) return;

        if (left == hPtr) {
            (*right)->startH = true;
        }
        else if (right == hPtr) {
            (*left)->endH = true;
        }
        else {
            (*right)->startH = (*left)->startH;
            (*right)->word = (*left)->word + (*right)->word;
            (*left) = (*right);
        }

        if ((*right)->startH && (*right)->endH) {
            cout << "RESULT: " << (*right)->word << endl;
        }
        else if ((*left)->startH && (*left)->endH) {
            cout << "RESULT: " << (*left)->word << endl;
        }
    };


    unordered_map<int, Interval**> intervals;

    for (auto pair : sequence) {
        int index = pair.first;
        char c = pair.second;
        cout << "----" << index << ", " << c << "-----" << endl;
        
        int preIndex = index - 1;
        int postIndex = index + 1;
        auto it_pre = intervals.find(preIndex);
        auto it_post = intervals.find(postIndex);
        
        //HERE 1
        //if (it_pre != intervals.end()) cout << (*(it_pre->second))->word << endl;
        
        Interval** currentIntervalPtr;

        if (c == '-') {
            currentIntervalPtr = hPtr;
        }
        else {
            Interval* currentInterval = new Interval(string(1, c));
            currentIntervalPtr = &currentInterval;
        }

        //HERE 2
        //if (it_pre != intervals.end()) cout << (*(it_pre->second))->word << endl;

        if (it_pre != intervals.end()) {
            Interval** prePtr = it_pre->second;
            //cout << "PRE: <" << (*currentIntervalPtr)->word << ", " << (*prePtr)->word << ">>" << endl;
            consolidate(prePtr, currentIntervalPtr);
        }

        if (it_post != intervals.end()) {
            Interval** postPtr = it_post->second;
            //cout << "POST: <" << (*currentIntervalPtr)->word << ", " << (*postPtr)->word << ">>" << endl;
            consolidate(currentIntervalPtr, postPtr);
        }
        
        intervals[index] = currentIntervalPtr;
    }

    return 0;
}

Between my two print statements (HERE 1 and HERE 2), the value of (*(it_pre->second))->word changes. All I've done between these statements is create a new, unrelated Interval, so I don't understand why my map values are changing.

Any insights would be appreciated. I'm new to using double pointers, so I might just be overlooking something simple.

Answer 1

Here:

        else {
            Interval* currentInterval = new Interval(string(1, c));
            currentIntervalPtr = &currentInterval;
        }

... you are assigning to currentIntervalPtr the address of a variable local to that block. The lifetime of that pointed-to pointer variable ends when execution of the block terminates, after which dereferencing currentIntervalPtr produces undefined behavior.

Note well that it doesn't matter that the Interval itself lives on. Its lifetime is entirely separate from and independent of the lifetime of any pointer pointing to it.

I'm not quite tracking what you're trying to gain with the double pointers here, but perhaps instead of a pointer to a block-scope variable, you can store the Interval pointers in a vector declared outside the loop, and use pointers to the elements of that vector. But I'm also thinking that instead of a vector<Interval *> and double pointers, you could probably make it easier on yourself with a vector<Interval> and single pointers -- that assuming you don't want to engage in a deeper refactoring.

Answer 2

Both John Bollinger's answer and Jun H's answer identify the specific cause of the bug in your program correctly. But - I would say the origin of your problem is your use of pointers.

You should not use pointers unless you actually need them.

It's not just me saying this; see:

• C++ Core Guideline R.5: Prefer scoped objects, don’t heap-allocate unnecessarily.
• this useful explanation on the Software Engineering Stack Exchange.
• this polemic presentation of typical cases where people use pointers, but don't need to.

You really don't need pointers here:

• When you want a new interval, just create one the simple and regular way: Interval(my_string); and pass it or assign it wherever you need to. Doing this will also work well with returning Intervals from functions rather than passing in pointers to intended result locations.
• When you want to refer to an existing interval (which you should also avoid doing overmuch), use a C++ reference to wherever it is you found it.

This would also help make your code simpler and more legible. Right now, with you repeatedly changing already-inserted intervals, it's not clear what the intervals map even holds.

---

Minor nitpicks:

• If intervals is indexed by int value, and you iterate int values sequentially, why not just use an std::vector<Interval>?

• You should avoid using namespace std, and just do using on what you actually use. See:
  Why is "using namespace std;" considered bad practice?