I'm having an issue with linked list of structures in C-CodePudding

I have to create a linked list of structures Book, my problem is when I create a new node and enter the properties of a new book inside of it, all of my previous books in previous nodes change their names to that last one I entered, although the year of publishing remains original. Can someone explain how this works, and what I have to do to make it work properly?

Here is my code

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

#define N 255

struct Book {
    char *title;
    int year;
};

struct Node {
    struct Book book;
    struct Node *next;
};

struct Book createBook(char *title, int year) {
    struct Book book = {title, year};
    return book;
}

void print(struct Node *head) {
    struct Node *curr_node = head;

    while (curr_node != NULL) {
        printf("%s(%d)\n", curr_node->book.title, curr_node->book.year);
        curr_node = curr_node->next;
    }
}

void add(struct Node **head, struct Book book) {
    struct Node *new_node;
    struct Book *new_book;
    new_node = (struct Node*) malloc(sizeof( struct Node ));

    new_book = &new_node->book;

    new_book->title = book.title;
    new_book->year = book.year;

    new_node->next = (*head);
    *head = new_node;
}

void pop(struct Node **head) {
    struct Node *next_node = NULL;

    if (*head == NULL) {
        printf("List is empty!");
        return;
    }

    next_node = (*head)->next;
    free(*head);
    *head = next_node;
}

void reverse(struct Node **head) {
    struct Node *prev_node = NULL;
    struct Node *curr_node = *head;
    struct Node *next_node = NULL;

    while (curr_node != NULL) {
        next_node = curr_node->next;
        curr_node->next = prev_node;
        prev_node = curr_node;
        curr_node = next_node;
    }

    *head = prev_node;
}

void clear(struct Node **head) {

    struct Node *curr_node = *head;
    struct Node *temp_node = NULL;

    while (curr_node != NULL) {
        temp_node = curr_node->next;
        free(curr_node);
        curr_node = temp_node;
    }

    *head = NULL;
}

int main() {

    struct Node *head = NULL;

    short process = 1; // 1 for True 0 for False
    int option = 0;
    char title[N];
    int year = 0;

    while (process) {

        printf("List of actions:\n");
        printf("1:\tAdd element\n");
        printf("2:\tRemove element\n");
        printf("3:\tReverse list\n");
        printf("4:\tPrint list\n");
        printf("5:\tClear list\n");
        printf("0:\tExit\n");

        scanf("%d", &option);

        switch (option) {

            case 1:
                printf("Enter book\'s title: ");
                scanf("%s", title);
                printf("Enter book\'s year of publishing: ");
                scanf("%d", &year);
                add(&head, createBook(title, year));
                printf("\n");
                break;
            case 2:
                pop(&head);
                break;
            case 3:
                reverse(&head);
                break;
            case 4:
                print(head);
                break;
            case 5:
                clear(&head);
                break;
            case 0:
                process = 0;
                break;
            default:
                printf("\n");
                printf("**************************\n");
                printf("Enter numbers from 1 to 5!\n");
                printf("**************************\n");
                printf("\n");
                continue;
        }
    }
    return 0;
}

CodePudding user response：

You are always passing a pointer to the same local array title declared in main

 char title[N];

to the function createBook

add(&head, createBook(title, year));

So the data member title of all nodes

struct Book {
    char *title;
    int year;
};

points to the same local array title declared in main due to this initialization within the function

struct Book book = {title, year};

That is the local array title declared in main used as an argument expression of the function createBook is implicitly converted to pointer to its first element. And all function calls deal with the same address of the first element of the array.

You need to allocate dynamically a copy of the referenced array title for each node.

Another approach is to declare the data member title of the structure Book as having a character array type instead of the pointer type.

CodePudding user response：

Thanks, now everything works!

I allocated memory for my char pointer in while loop in main function right before switch statement:

char *title = (char *) calloc(N, sizeof(char));

So now the working code looks like this:

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

#define N 255

struct Book {
    char *title;
    int year;
};

struct Node {
    struct Book book;
    struct Node *next;
};

struct Book createBook(char *title, int year) {
    struct Book book = {title, year};
    return book;
}

void print(struct Node *head) {
    struct Node *curr_node = head;

    while (curr_node != NULL) {
        printf("%s(%d)\n", curr_node->book.title, curr_node->book.year);
        curr_node = curr_node->next;
    }
}

void add(struct Node **head, struct Book book) {
    struct Node *new_node;
    struct Book *new_book;
    new_node = (struct Node*) malloc(sizeof( struct Node ));

    new_book = &new_node->book;

    new_book->title = book.title;
    new_book->year = book.year;

    new_node->next = (*head);
    *head = new_node;
}

void pop(struct Node **head) {
    struct Node *next_node = NULL;

    if (*head == NULL) {
        printf("List is empty!");
        return;
    }

    next_node = (*head)->next;
    free(*head);
    *head = next_node;
}

void reverse(struct Node **head) {
    struct Node *prev_node = NULL;
    struct Node *curr_node = *head;
    struct Node *next_node = NULL;

    while (curr_node != NULL) {
        next_node = curr_node->next;
        curr_node->next = prev_node;
        prev_node = curr_node;
        curr_node = next_node;
    }

    *head = prev_node;
}

void clear(struct Node **head) {

    struct Node *curr_node = *head;
    struct Node *temp_node = NULL;

    while (curr_node != NULL) {
        temp_node = curr_node->next;
        free(curr_node);
        curr_node = temp_node;
    }

    *head = NULL;
}

int main() {

    struct Node *head = NULL;

    short process = 1; // 1 for True 0 for False
    int option = 0;

    int year = 0;

    while (process) {

        printf("List of actions:\n");
        printf("1:\tAdd element\n");
        printf("2:\tRemove element\n");
        printf("3:\tReverse list\n");
        printf("4:\tPrint list\n");
        printf("5:\tClear list\n");
        printf("0:\tExit\n");

        scanf("%d", &option);
        char *title = (char *) calloc(N, sizeof(char));
        switch (option) {

            case 1:

                printf("Enter book\'s title: ");
                scanf("%s", title);
                printf("Enter book\'s year of publishing: ");
                scanf("%d", &year);
                add(&head, createBook(title, year));
                printf("\n");
                break;
            case 2:
                pop(&head);
                break;
            case 3:
                reverse(&head);
                break;
            case 4:
                print(head);
                break;
            case 5:
                clear(&head);
                break;
            case 0:
                process = 0;
                break;
            default:
                printf("\n");
                printf("**************************\n");
                printf("Enter numbers from 1 to 5!\n");
                printf("**************************\n");
                printf("\n");
                continue;
        }
    }
    return 0;
}