Caligula
Caligula

Reputation: 15

Can't add new node to linked list

I'm trying to write a function that adds new node at the end of likded list and the program terminates for unknown reasons. I've added "Works for now" stamps to visualise what's going on. it looks like the program terminates right before the if (*first == NULL) {...}

Code:

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

struct student
{
    int ID;
    char name[50];
    char surname[50];
    struct student *next;
};

void insertattheend(struct student **first)
{
    struct student *new = (struct student *)malloc(sizeof(struct student));
    struct student *last = *first;
    int Idtemp;
    char Nametemp[50], Surnametemp[50];
    printf("ID: ");
    scanf("%d", &Idtemp);
    printf("Name: ");
    scanf("%s", Nametemp);
    printf("Surname: ");
    scanf("%s", Surnametemp);
    new->ID = Idtemp;
    int i;
    for (i = 0; i < 50; i++)
    {
        new->name[i] = Nametemp[i];
        new->surname[i] = Surnametemp[i];
    }
    printf("Works for now\n");
    new->next = NULL;
    printf("Works for now\n");
    if (*first == NULL)
    {
        printf("Works for now\n");
        *first = new;
        printf("Works for now\n");
        return;
    }

    while (last->next != NULL)
        last = last->next;

    last->next = new;
    return;
}

void printlist(struct student *oof)
{
    while (oof != NULL)
    {
        printf("ID: %d\nName: %s\nSurname: %s ", oof->ID, oof->name, oof->surname);
        oof = oof->next;
    }
}

int main(void)
{
    struct student *head = NULL;
    head = (struct student *)malloc(sizeof(struct student));
    char operation;
    // a - insert
    printf("Choose operation: ");
    scanf("%c", &operation);
    if (operation == 'a')
        insertattheend(&head); // add element

    printlist(head);
    return 0;
}

Output

Choose operation: a
ID: 12
Name: John
Surname: Nhoj
Works for now
Works for now

I've tried everything i could've think of. Does anybody know how to make this work? Thanks for help

Upvotes: 1

Views: 53

Answers (1)

WhozCraig
WhozCraig

Reputation: 66234

First your head insertion is needlessly complex. You can significantly shorting it by reading into a temporary student rather than a raft of otherwise-unrelated buffers. A semi-complete example appears below:

void insertattheend(struct student **first)
{
    // this is where we'll read our data first
    struct student s = {0};

    // TODO: ALL of these need proper error checking to ensure
    //  they succeeded
    printf("ID: ");
    scanf("%d", &s.ID);
    printf("Name: ");
    scanf("%49s", s.name);
    printf("Surname: ");
    scanf("%49s", s.surname);

    // march 'first' down the pointer chain until it addresses 
    // the last pointer in the list (which wil be the 'head' 
    // pointer itself if the list is empty, or the 'next'
    // member of the last node if the list is not empty.
    while (*first)
        first = &(*first)->next;

    // allocate a new node
    *first = malloc(sizeof **first);
    if (*first == NULL)
    {
        perror("Failed to allocate new list node");
        exit(EXIT_FAILURE);
    }

    // structure copy 's' into the location pointed to by
    //  the pointer pointed to by 'first'
    **first = s;

    // done
}

Second, your main is needlessly allocating space for a node pointed to by head, when in reality it should simply start as NULL and allow the insertion function to manage it from there. The semi-full program appears below. I suspect you'll encounter other problem, some of which I've subtly addressed here.

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

struct student
{
    int ID;
    char name[50];
    char surname[50];
    struct student *next;
};

void insertattheend(struct student **first)
{
    // this is where we'll read our data first
    struct student s = {0};

    // TODO: ALL of these need proper error checking to ensure
    //  they succeeded
    printf("ID: ");
    scanf("%d", &s.ID);
    printf("Name: ");
    scanf("%49s", s.name);
    printf("Surname: ");
    scanf("%49s", s.surname);

    // march 'first' down the pointer chain until it addresses 
    // the last pointer in the list (which wil be the 'head' 
    // pointer itself if the list is empty, or the 'next'
    // member of the last node if the list is not empty.
    while (*first)
        first = &(*first)->next;

    // allocate a new node
    *first = malloc(sizeof **first);
    if (*first == NULL)
    {
        perror("Failed to allocate new list node");
        exit(EXIT_FAILURE);
    }

    // structure copy 's' into the location pointed to by
    //  the pointer pointed to by 'first'
    **first = s;

    // done
}

void printlist(const struct student *oof)
{
    while (oof != NULL)
    {
        printf("ID: %d\nName: %s\nSurname: %s\n", oof->ID, oof->name, oof->surname);
        oof = oof->next;
    }
}

void deletelist(struct student *lst)
{
    while (lst)
    {
        struct student *tmp = lst;
        lst = lst->next;
        free(tmp);
    }
}

int main(void)
{
    struct student *head = NULL;
    char operation;

    printf("Choose operation: ");
    scanf(" %c", &operation);
    if (operation == 'a')
        insertattheend(&head); // add element

    printlist(head);
    deletelist(head);
    return 0;
}

Upvotes: 2

Related Questions