I am unable to display or insert correctly elements in singly circular linked list

Question

I am unable to insert and display in singly circular linked list, It only inserts first elements and same with display function

void create() {
    struct node *temp , *p;
    temp = (struct node*)malloc(sizeof(struct node));
    printf("Enter node data :");
    scanf("%d",&(temp->data));
    temp->next = NULL;
    if(root == NULL) {
        root = p = temp;
    }
    else {
        while(p->next!=root) {      
            p->next = temp;
            p = temp;
            p->next = root;
        }
    }
}

void display() {
    struct node *temp;
    if(root == NULL) {
        printf("List is Empty\n");
    }
    else {
        temp = root;
        while(temp->next!=root) {
            printf("%d",temp->data);
            temp = temp->next;
        }
        printf("%d",temp->next);
    }
}

Answer 1

In this part of the create function:

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

variable p is not initialized, so p->next is an invalid dereference.

Changing it as follows:

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

corrects the uninitialized variable. The code is still incorrect because it discards the existing elements on the list, so the list contains only the new element.

The following changes it to look for the last element of the list, and then appends the new element to the last element:

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

Then it is necessary to link the new element (which is now the last element) to the first element to complete the circle:

        temp->next = root;

For the case when the list is empty (i.e. root == NULL), after making root point to the new node (i.e. root = temp;), it is also necessary to make the new node point to itself, since it is also the first node:

        temp->next = temp;

or:

        temp->next = root;

The temp->next = root; statement can be moved to be after the if else statement since it is at the end of both branches.

Here is a slightly cleaned up version of the create function. I have not added any error checking for the result of the malloc and scanf call, since the main focus of the question and answer is to do with the circular linked list handling:

void create(void) {
    struct node *temp , *p;
    temp = (struct node*)malloc(sizeof(struct node));
    printf("Enter node data :");
    fflush(stdout);
    scanf("%d",&(temp->data));
    if(root == NULL) {
        /* list is empty so new node will become both first and last node */
        root = temp;
    }
    else {
        /* find the current last node */
        for (p = root; p->next != root; p = p->next)
            ;
        /* append the new node so it will become last */
        p->next = temp;
    }
    /* link the new, last node to the first node */
    temp->next = root;
}

The existing display function should now work, but it can be simplified a bit with the use of a do while loop for the non-empty case:

void display(void) {
    struct node *temp;
    if(root == NULL) {
        printf("List is Empty\n");
    }
    else {
        temp = root;
        do {
            printf("%d ",temp->data);
            temp = temp->next;
        } while (temp!=root);
        printf("\n");
    }
}

It is not that much simpler, and still has two printf statements (although the last one could be changed to putchar('\n');). It also prints a space before the newline, so you may want to deal with the final element of the list separately after all:

void display(void) {
    struct node *temp;
    if(root == NULL) {
        printf("List is Empty\n");
    }
    else {
        for (temp = root; temp->next != root; temp = temp->next) {
            printf("%d ",temp->data);
        }
        printf("%d\n", temp->data);
    }
}