why am I getting out of bounds error in this linked list? C

Question

So am making a linked list. printing it out. and reversing it. and then printing it out. first time I make it and print it out. everything works fine. but when I reverse it. it reverses successfully. but when I print it. I go out of bounds even though I use the same code I did first.

Here is the reverse function

void reverse_list(Node_ptr* head){

Node_ptr temp2;
Node_ptr temp3 = NULL;
temp2 = (Node_ptr)malloc(sizeof(Node));
temp3 = (Node_ptr)malloc(sizeof(Node));

if (temp2==NULL || temp3==NULL)
{
    printf("Failed to allocate node\n");
    exit(1);
}

while (*head!=NULL) {
     temp2 = (*head)->next;
    (*head)->next = temp3;
    temp3 = (*head);
    (*head) = temp2;
}
 *head = temp3;

}

here is the print function

temp = head;
while (temp != NULL)
{
    printf("%d\n", temp->data);
    temp = temp->next;
}

reverse_list(&head);

temp = head;

while (temp != NULL)
{
    printf("%d\n", temp->data);
    temp = temp->next;
}

for some reason it tries to print garbage after the last element

Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, Jun 01 '15 at 07:43
Also, please post the related structure and typedefs along with your code, don't let us guess them. — Sourav Ghosh, Jun 01 '15 at 07:44

score 5 · Answer 1 · answered Jun 01 '15 at 07:48

Do this:

/* Function to reverse the linked list */
void reverse(struct node** head_ref)
{
    struct node* prev   = NULL;
    struct node* current = *head_ref;
    struct node* next;

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

    *head_ref = prev;
}

It's actually your code with a couple of fixtures, i.e.:

1) You don't need to allocate space, just swap pointers.

2) Use meaningful names for your temporary containers.

score 1 · Answer 2 · answered Jun 01 '15 at 07:49

1

The first time you go through the loop

while (*head!=NULL) {
     temp2 = (*head)->next;
    (*head)->next = temp3;
    temp3 = (*head);
    (*head) = temp2;
}

(*head)->next is assigned a newly allocated node. Who knows what this node contains? It is probably not zeroed out, and will point to a random point in memory.

You should initialize temp3 to NULL to fix this problem.

answered Jun 01 '15 at 07:49

John M

1,484
1
14
27

Let me clarify myself a little better. You should delete the line saying temp3 = malloc(...); The value of temp3 should be null when the while loop starts. – John M Jun 01 '15 at 08:01

why am I getting out of bounds error in this linked list? C

2 Answers2