Doubly linked list FM station implementation

Question

The Question is:

Create a bidirectional linked list of FM stations (mentioned below) using C. Then set pointer to first station. Take user input: Enter 1 to play next station Enter 2 to play previous station Enter 3 to play first station Enter 4 to exit

The linked list for FM station related data should be:

Station 1 Radio_City 91.1 Station 2 Radio_One 91.8 Station 3 BIG_FM 92.7 Station 4 RED_FM 93.5 Station 5 My_FM 94.3 Station 6 Hit_FM 95.0 Station 7 Radio_Mirchi 98.3 Station 8 Vividh_Bharti 100.1 Station 9 Gyan_Vani 105.6 Station 10 Radio_Dhamal 106.4 Station 11 DD_News 108.2
Station 12 Gyan_Ganga 109.6

The code that I have written is:

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

struct Node
{
    int sno;
    char sname[50];
    float sfreq;
    struct Node *Lnext,*Rnext;
};

struct Node *createnewnode()
{
    int n;
    struct Node *first,*last;
    printf("\n Enter the number of stations available:");
    scanf("%d",&n);
    first=(struct Node*)malloc(sizeof(struct Node));
    printf("\nEnter the Station number: ");
    scanf("%d",&first->sno);
    printf("Enter the Station Name: ");
    fflush(stdin);
    scanf("%s",first->sname);
    printf("Enter the station Frequency: ");
    scanf("%f",&first->sfreq);
    last=first;
    last->Lnext=NULL;
    last->Rnext=NULL;
  
    int i=2;
    while(i<=n)
    {
        struct Node *temp=(struct Node*)malloc(sizeof(struct Node));
        printf("\nEnter the Station number: ");
        scanf("%d",&temp->sno);
        printf("Enter the station name: ");
        fflush(stdin);
        scanf("%s",temp->sname);
        printf("Enter the station frequency: ");
        scanf("%f",&temp->sfreq);
        
        temp->Lnext=last;
        last->Rnext=temp;
        last=last->Rnext;
        last->Rnext=NULL;
        
        i++;
    }
    return first;
}

void display_by_position(struct Node *first)
{
    struct Node *temp=first;
    printf("\n The list of stations is....\n");
    if(temp==NULL)
    {
        printf("\nThere are no stations to display");
        return;
    }
    else
    {
        printf("\n\tSTATION NUMBER\t\tSTATION NAME\t\tSTATION FREQUENCY");
        for(temp=first;temp!=NULL;temp=temp->Rnext)
        {
            printf("\n\t%d\t\t\t%s\t\t\t%.1f",temp->sno,temp->sname,temp->sfreq);
        }
    }
    
    int choice;
    while(choice!=4)
    {
        printf("\n-------------------------------------------\n");
        printf("\nEnter \"1\" to play the next station");
        printf("\nEnter \"2\" to play the previous station");
        printf("\nEnter \"3\" to play the first station");
        printf("\nEnter \"4\" to exit");
        printf("\n-------------------------------------------\n");
        printf("\nEnter your choice: ");
        scanf("%d",&choice);
        
        if(choice==1)
        {
            if(temp->Rnext==NULL)
            printf("\nThis is the last station\n");
            else
            {
                temp=temp->Rnext;
                printf("\n The next station is...\n");
                printf("\nStationNumber:  %d      %s      %f\n",temp->sno,temp->sname,temp->sfreq);
            }
        }
        
        else if(choice==2)
        {
            if(temp->Lnext==NULL)
            printf("\nThe is the first station");
            else
            {
                temp=temp->Lnext;
                printf(" \n The previous station is...\n");
                printf("\nStationNumber:  %d      %s      %f\n",temp->sno,temp->sname,temp->sfreq);
            }
        }
        
        else if(choice==3)
        {
            temp=first;
            printf("\n The first station in the list is...\n");
            printf("\nStationNumber:  %d      %s      %f\n",temp->sno,temp->sname,temp->sfreq);
        }
        
        else if(choice==4)
        {
            printf("\n Thank you for your patience...");
        }
        
        else
        {
            printf("\n Invalid choice...");
        }
    }
}

int main()
{
    struct Node *p=createnewnode();
    display_by_position(p);
    return 0;
}

In this code, the program gets terminated when the user gives input as '1' to play the next station. Similarly, the program gets terminated for whatever input the user gives

Answer 1

While you initialize temp with struct Node *temp=first;, you subsequently use temp as a loop variable to show the list of stations:

               for(temp=first;temp!=NULL;temp=temp->Rnext)
                  {
                          printf("\n\t%d\t\t\t%s\t\t\t%.1f",temp->sno,temp->sname,temp->sfreq);
                  }

which means that temp == NULL when the loop exits. When you subsequently press '1' it does temp->Rnext which will dereference a NULL pointer and your program crashes.

You have to be really careful when reusing variables like this, so it's usually much safer to create a local variable when needed:

               for(struct Node *temp=first;temp!=NULL;temp=temp->Rnext)
                  {
                          printf("\n\t%d\t\t\t%s\t\t\t%.1f",temp->sno,temp->sname,temp->sfreq);
                  }

You should move struct Node *temp=first; to before your choice-loop, say, after int choice; otherwise your compiler might complain that variable is being shadowed.

Breaking your program into smaller functions will also help you isolate parts of your program so you only have to reason about each part instead of the entire program all at once. For example you could extract the list of stations like this:

int list(struct Node *first) {
    printf("\n The list of stations is....\n");
    if(!first) {
        printf("\nThere are no stations to display");
        return 1;
    }
    printf("\n\tSTATION NUMBER\t\tSTATION NAME\t\tSTATION FREQUENCY");
    for(struct Node *t=first; t!=NULL; t=t->Rnext) {
        printf("\n\t%d\t\t\t%s\t\t\t%.1f", t->sno, t->sname, t->sfreq);
    }
    return 0;
}

void display_by_position(struct Node *first) {
    if(list(first))
       return;
    ...
}

Change your while(choice != 4) { ... } to a do { ... } while(choice != 4) so you are not checking the value of an uninitialized variable.

You are repeating yourself in createnewnode() so is my suggestion to streamline it:

struct Node *createnewnode() {
    int n;
    printf("\n Enter the number of stations available:");
    scanf("%d",&n);

    struct Node *first = NULL;
    struct Node *last = NULL;
    while(n-- > 0) {
        struct Node *temp = malloc(sizeof(struct Node));
        if(!temp) {
            printf("malloc failed\n");
            return NULL;
        }

        printf("\nEnter the Station number: ");
        scanf("%d", &temp->sno);

        printf("Enter the station name: ");
        scanf("%49s", temp->sname);

        printf("Enter the station frequency: ");
        scanf("%f", &temp->sfreq);

        temp->Lnext = last ? last : NULL;
        temp->Rnext = NULL;

        if(!first) first = temp;

        if(last) last->Rnext = temp;
        last = temp;
    }
    return first;
}

You may want to check the return code of scanf, for example, if you enter "a" for station number it will not work the way you expect: