Search code examples
clinked-list

Insert function for a sorted linked list


I wrote a program that gets some value and puts it in a sorted linked list. The problem is that after entering the first value the program stops and it doesn't even run the insert function

I know the problem is in passing argument to the insert function But I don't know how to fix it.

#include <stdio.h>
#include <stdlib.h>

struct record{
    int data;
    struct record *nextptr;
};

void insert (struct record *ptr,int value);
void printList(struct record *ptr);

int main() {
    struct record *headptr = NULL;
    for (int i = 0; i < 4; ++i) {
        int data;
        printf("Enter your value");
        scanf("%d", &data);
        insert(headptr, data);
    }
    printList(headptr);


    return 0;
}

void insert (struct record *ptr,int value){
    printf("WE ARE IN THE INSERT FUNCTION");
    struct record *newptr = (struct record *) malloc(sizeof(struct record));
    newptr->data=value;
    newptr->nextptr=NULL;
    struct record *curptr;
    curptr=ptr;

    while(value >= (curptr->data)){
        curptr=curptr->nextptr;
    }
    if (curptr==NULL){
        ptr=newptr;
    }
    else{
        newptr->nextptr=curptr;
    }

}

void printList(struct record *ptr){
    while((*ptr).nextptr != NULL){
        printf("%d", ptr->data);
        ptr=ptr->nextptr;
    }
}

Result:

/Users/Danial/CLionProjects/Example/cmake-build-debug/Example
Enter your value3

Process finished with exit code 11


Solution

  • You have two big problems with insert. First you pass ptr as a pointer struct record. When that happens, your insert function receives a copy-of-the-pointer that points to the same location in memory, but has a distinct address itself, very different from the pointer passed from main(). What this means is not matter what you do to the copy of the pointer in insert, those changes will never be seen back in main() because you are operating on a copy and not otherwise returning a value. To cure that issue make your parameter struct record ** and pass the address of the pointer from main().

    The next biggest problem you have in insert is you fail to check whether curptr is NULL (as it will be on the first call to insert) before you begin dereferencing curptr -- likely leading to a segfault or other run of the mill Undefined Behavior.

    When creating a linked list, you have two distinct test conditions you must handle:

    1. Am I inserting the first node? (if so, just assign `*ptr = newptr); and
    2. all other insertions will require you to iterate to find the proper 2-nodes to insert your data between based on the value of the data.

    You can handle both cases simply:

        rec_t *curptr = *ptr,
            *newptr = malloc (sizeof *newptr);
        if (!newptr) {
            perror ("malloc-newptr");
            return;
        }
    
        newptr->data = value;
        newptr->nextptr = NULL;
    
        if (curptr == NULL) {       /* handle new-list case and return */
            *ptr = newptr;
            return;
        }
    
        if (value < curptr->data) {     /* handle new 1st node */
            newptr->nextptr = curptr;
            *ptr = newptr;
            return;
        }
    
        /* iterate with curptr until value > curptr->nextptr->data */
        while (curptr->nextptr != NULL && value > curptr->nextptr->data)
            curptr = curptr->nextptr;
    
        newptr->nextptr = curptr->nextptr;      /* wire new node to next node */
        curptr->nextptr = newptr;               /* wire current to new node */
    

    In any program you write that allocates memory, you must preserve a pointer to the start of each block allocated so it can be freed when that memory is no longer needed. You will generally want to write a listfree function to handle that for you. You could write something simple like the following:

    void freelist (rec_t *head)
    {
        while (head) {
            rec_t *victim = head;
            head = head->nextptr;
            free (victim);
        }
    }
    

    (note: how the node to free is saved in a temporary pointer victim before the list is advanced to the next node).

    Putting it altogether, you could do something like the following:

    #include <stdio.h>
    #include <stdlib.h>
    
    typedef struct record {
        int data;
        struct record *nextptr;
    } rec_t;
    
    void insert (rec_t **ptr, int value);
    void printList (rec_t *ptr);
    void freelist (rec_t *head);
    
    int main (void) {
        rec_t *headptr = NULL;
    
        for (int i = 0; i < 4; ++i) {
            int data;
            printf("Enter your value: ");
            scanf("%d", &data);
            insert (&headptr, data);
        }
        printList(headptr);
        freelist (headptr);
    
        return 0;
    }
    
    void insert (rec_t **ptr, int value) 
    {
        if ( ptr == NULL ) {
            return;  
        } 
    
        rec_t *curptr = *ptr,
            *newptr = malloc (sizeof *newptr);  /* don't cast the return */
        if (!newptr) {
            perror ("malloc-newptr");
            return;
        }
    
        newptr->data = value;
        newptr->nextptr = NULL;
    
        if (curptr == NULL) {       /* handle new-list case and return */
            *ptr = newptr;
            return;
        }
    
        if (value < curptr->data) {     /* handle new 1st node */
            newptr->nextptr = curptr;
            *ptr = newptr;
            return;
        }
    
        /* iterate with curptr until value > curptr->nextptr->data */
        while (curptr->nextptr != NULL && value > curptr->nextptr->data)
            curptr = curptr->nextptr;
    
        newptr->nextptr = curptr->nextptr;      /* wire new node to next node */
        curptr->nextptr = newptr;               /* wire current to new node */
    }
    
    void printList(struct record *ptr)
    {
        while (ptr != NULL) {
            printf(" %d", ptr->data);
            ptr = ptr->nextptr;
        }
        putchar ('\n');
    }
    
    void freelist (rec_t *head)
    {
        while (head) {
            rec_t *victim = head;
            head = head->nextptr;
            free (victim);
        }
    }
    

    Example Use/Output

    $ ./bin/lltcmpl
    Enter your value: 1
    Enter your value: 8
    Enter your value: 5
    Enter your value: 7
     1 5 7 8
    
    $ ./bin/lltcmpl
    Enter your value: 2
    Enter your value: 1
    Enter your value: 4
    Enter your value: 3
     1 2 3 4
    

    Look things over and let me know if you have questions.