segmentation fault when using pointer to pointer C++

Question

I have been trying to use pointer to pointer to perform action something similar to inorder traversal in tree. here is my code

struct node {
    char c = '\0';
    int freq = 0;
    node *left = NULL;
    node *right = NULL;
    string code = "";
};

void appendones(node **n) {
    if ((*n) == NULL)
        ;
    else {
        (*n)->code += "1";
        appendones(&(*n)->left);
        appendones(&(*n)->right);
    }
}

void combinenodes(node *a, node *b, node **n) {
    appendones(&a);
    appendones(&b);
    //(*n)=newNode('\0',a->freq+b->freq);
    (*n)->c = '\0';
    (*n)->freq = a->freq + b->freq;
    (*n)->left = a;
    (*n)->right = b;
}

int main() {
    N = input();
    priority_queue<node, vector<node *>, compareNodefreq> nodefreq; // function object
    for (int i = 0; i < N; i++) {
        char s;
        int freq;

        cin >> s >> freq;
        node *n = newNode(s, freq);
        nodefreq.push(n);
    }

    // printheap(nodefreq);

    // perform combining nodes based on frequencies
    while (nodefreq.size() > 1) {
        node *a = nodefreq.top();
        nodefreq.pop();
        node *b = nodefreq.top();
        nodefreq.pop();

        node *n;
        combinenodes(a, b, &n);
        nodefreq.push(n);
    }
}

I get segmentation fault in (*n)->code+="1"; for appendone().

I am not able to figure out the error. My understanding is that I am passing the pointer by reference and performing the appendzero() and appendone(), so I guess there is no error in that part. Also my a and b in combinenodes() cannot be null because I am popping from the stack.

Could you help me figure out?

And try to debug the code by yourself at first. These kind of errors are usually easy to be detected by a debugger. — Jepessen, Nov 09 '15 at 20:31
Could you post the full exact segfault message? And `newNode()`? — donjuedo, Nov 09 '15 at 21:12
Use a debugger and read a [book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) about C++. No `new` for you. — , Nov 09 '15 at 21:43

donjuedo · Answer 1 · 2015-11-09T21:34:32.277

0

Oh, I see it. In main()'s while loop, n is pushed and used later, but is invalid because the node object itself is never allocated in combinenodes() (currently commented out). The pointer value, then, is undefined, but in this case, turning out to be non-zero, defeating the safety check.

edited Nov 09 '15 at 21:34

answered Nov 09 '15 at 21:16

donjuedo

2,475
18
28

That's likely not the only problem. The queue declaration itself seems to implore the end of innocence among other things... http://paste.ubuntu.com/13211286/ – sehe Nov 09 '15 at 21:43
Indeed. Right next to that TODO you refer to, is a `node*` mention, whereas the code above uses `node`. Still, I'm not familiar with `priority_queue` type, and execution got past pushing to reach the segfault. The OP reply will be interesting. – donjuedo Nov 09 '15 at 21:54

segmentation fault when using pointer to pointer C++

1 Answers1