Error Cannot access memory at address in linked list

Question

I'm creating a linked list with some insert functions.

This method work fine :

void insert_before(){
    struct node *new_node, *ptr, *preptr;
    int pivot;

    new_node = (struct node*)malloc(sizeof(struct node));
    ptr = link;
    preptr = link;

    printf("\n Enter the data : ");
    scanf("%d", &new_node->data);

    printf("\n Enter the value before which the data has to be inserted : ");
    scanf("%d", &pivot);

    while(ptr->data != pivot){
        preptr = ptr;
        ptr = ptr->next;
    }

    preptr->next = new_node;
    new_node->next = ptr;
}

However, I got Cannot access memory at address 0x8 in this method :

void insert_after(){
    struct node *new_node, *ptr, *preptr;
    int pivot;

    new_node = (struct node*)malloc(sizeof(struct node));
    ptr = link;
    preptr = link;

    printf("\n Enter the data : ");
    scanf("%d", &new_node->data);

    printf("\n Enter the value after which the data has to be inserted : ");
    scanf("%d", &pivot);

    while(preptr->data != pivot){
        preptr = ptr;
        ptr = ptr->next;
    }

    preptr->next = new_node;
    new_node->next = ptr;
}

Note that both of the method use the same struct* link, and the only difference is on the looping while(preptr->data != pivot).

Why does the first code working fine, but the second one broken?

Thanks for your help

PS : This is my whole project (very short), in case you need it : https://pastebin.com/wsMEicGv

What if `preptr` is a null pointer? Or if `pivot` isn't found? What happens then? Please learn how to use a debugger to catch crashes and examine the call stack. I also recommend you take some time to read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). — Some programmer dude, Jun 06 '17 at 14:00
@Stargateur You are right :) Its been a long time i dont code in C — Blaze Tama, Jun 07 '17 at 10:14
@Someprogrammerdude I purposely skip the validation parts because my goal is to learn how linked list algorithm work. But I do understand your point, I will try to learn to debug the errors in C (debugging java is much easier because of exceptions in my opinion). Thanks — Blaze Tama, Jun 07 '17 at 10:16

score 3 · Accepted Answer · answered Jun 06 '17 at 14:12

I don't think that even the first method works correctly (simple proof: the code never changes the value of link, hence an insert before the first node cannot work).

Anyway, your code will produce the mentioned error message in both functions when you enter a value for pivot which does not match any data or which matches the last node in the list:

while(preptr->data != pivot){
    preptr = ptr;
    ptr = ptr->next;
}

The reason is that the loop above will reach the end of the list (having preptr = NULL), when condition preptr->data still accesses this null-pointer. I suppose that member data is the second one in your struct (the first one is next), right? Then you access actually (NULL + sizeof(node*)) which is (NULL + 8) which is 0x08.

Thanks for your help, Yes, it seems the bug will start when the code try to find an int that not exists in the lists. However, its capable to insert after the last node of the list. Thanks for your help. — Blaze Tama, Jun 07 '17 at 10:12

Stargateur · Answer 2 · 2017-06-07T10:06:06.277

You have a lot of bad practice:

You use global without reason!
You use global without reason!!
You use global without reason!!!
You cast malloc() without reason.
You don't check return value of function like scanf() or malloc().
You code several time the same feature.
fflush(stdin); is undefined behavior.
You don't initialize your variable when you could.
You declare all your variable at the beginning of the function.

For example, display() could look like this:

void display(struct node *head) { // no global it's better, isn't it ?
    printf("List:");
    while (head != NULL) {
        printf(" %d", head->data);
        head = head->next;
    }
    printf("\n");
}

More, in your create_ll() function, why don't you use insert beginning and reverse the list after ?

int create_ll(struct node **head) {
    *head = NULL;
    while (insert_beginning(head) != ERROR) {
    }
    return reverse_ll(head);
}

Use example:

int main(void) {
    struct node *head;
    if (create_ll(&head) == ERROR) {
        return 1;
    }
    display(head);
    free_ll(head);
}

For the problem of your both function, read the answer of stephan-lechner. But I make you an example:

int insert_after(struct node **head) {
    printf("\n Enter the data : ");
    int data;
    if (scanf("%d", &data) != 1) { // If the input has no been parse.
        // data is invalid here
        return ERROR;
    }
    // data is valid here

    printf("\n Enter the value after which the data has to be inserted : ");
    int pivot;
    if (scanf("%d", &pivot) != 1) {
        return ERROR;
    }

    for (struct node *node = *head; node != NULL; node = node->next) {
        if (node->data == pivot) {
            struct node *new_node = malloc(sizeof *new_node);
            if (new_node == NULL) {
                return ERROR;
            }
            new_node->data = data;
            new_node->next = node->next;
            node->next = new_node;

            return OK;
        }
    }
    return ERROR;
}

Be aware that: If you use global your list functions will be:

Not thread safe.
Only usable for one list.

More info here.

Thanks a lot for your help and inputs. I use global variable because I try to think the linked list as `List<>` in java. Its a common practice that we have a list object in the class (global) and the function just use it without passing it as a param. If I do use the local variable via param, do I need to return it so all of my functions will `return node`? — Blaze Tama, Jun 07 '17 at 09:56
And what do you mean by `if (scanf("%d", &data) != 1) { return ERROR; }`? — Blaze Tama, Jun 07 '17 at 10:00
@BlazeTama You should read a [C book](https://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list). "If I do use the local variable via param, do I need to return it so all of my functions will `return node`?", yes and no, and like you see, I make you an example by using `struct node **head`, this can be use to update the pointer. But you can return it too. But return an int allow to return better error variation. Like `OK, ERROR_MALLOC, ERROR_INPUT`. It's a style question. — Stargateur, Jun 07 '17 at 10:07
Thanks, I will check the book. Could you explain a bit about **head? Is it a pointer of a pointer? — Blaze Tama, Jun 08 '17 at 06:10

Error Cannot access memory at address in linked list

2 Answers2