Adding a node to a Linked List gives a runtime error in C++

Question

I'm trying to create a function that will add a node at the end of a linked list. Every time I run it however, I'm getting a runtime error and have no idea where my mistake is. Here's my code:

Edge* append(Edge *head, int origin, int destination, int weight) {
    Edge *temp = new Edge;
    temp = head;
    Edge *node = new Edge;
    while (temp->next != NULL){
        temp = temp->next;
    }
    node->vert1 = origin;
    node->vert2 = destination;
    node->weight = weight;
    node->next = NULL;
    if (head == 0) {
        head = node;
    }
    else if (head != 0){
        node = temp;
    }
    return head;
}

UPDATE:

Here is the Edge struct:

struct Edge {
    int vert1;
    int vert2;
    int weight;
    Edge *next;
};

Here is the main function:

int main(){
    Edge *e = append(NULL, 1,4,5);
}

ideone.com/eN4MMG check this link. There is no runtime error — user2897690
– user2897690, Commented Feb 4, 2014 at 19:40
don't pass null to your function call in main. instead pass a NULL list in program. see code in my previous comment for more details — user2897690
– user2897690, Commented Feb 4, 2014 at 19:43

sigy · Accepted Answer · 2014-02-04 19:44:40Z

First thing I notice is that you check if head is NULL after you already dereferenced it. You set temp to head and then check if temp->next is NULL. If head was NULL this crashes. Furthermore your else if is redundant. If head was not equal to 0 than it must be different from 0. The third thing is that at the end you set node to temp but I think what you want to do is to set temp->next to node.

Edge* append(Edge *head, const int origin, const int destination, const int weight) {
    Edge *temp = head;

    Edge *node = new Edge;
    node->vert1 = origin;
    node->vert2 = destination;
    node->weight = weight;
    node->next = NULL;

    if (!head) {
        return node;
    }

    while (temp->next != NULL){
        temp = temp->next;
    }

    temp->next = node;

    return head;
}

EDIT Your code has a memmory leak, too. You never free the memory that you reserved for temp at the first line. However, there is no need to create a new Edge here.

tinkertime · Accepted Answer · 2014-02-04 19:52:34Z

while you declare:

Edge *temp = new Edge;

You then reassign temp to head:

temp = head;

Therefore, if you pass in a null head param, you will crash at

while (temp->next != NULL){

It seems like this reassignment isn't what you're actually looking to do, since the logic doesn't work out anyways beyond that. Try this:

Edge* append(Edge *head, int origin, int destination, int weight) {
    // create your new node
    Edge *node = new Edge;
    node->vert1 = origin;
    node->vert2 = destination;
    node->weight = weight;
    node->next = NULL;

    // if there is no head, this is the head
    if (head == NULL) {
        head = node;
        return node;
    }

    // else walk to the end and set this as as the new tail node
    // NOTE: this can be optimized by having your linkedlist implementation
    // store the pointer to the tail.
    Edge *temp = head;
    while (temp->next != NULL){
        temp = temp->next;
    }
    temp->next = node;

    return node;
}

Collectives™ on Stack Overflow

Adding a node to a Linked List gives a runtime error in C++

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related