C- Linked List with *chars

Question

I am trying to build a simple linked list.

I have successfully built a linked list only with int variables but when I add *char variables the output is wrong.

The int values seem to be correct but the *char type are wrong.

The *char type seem to always be the last one inserted.

Sample Input

Number: 5
Character: a

Number: 4
Character: b

Sample Output

5
b

**************

4
b

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

typedef struct BOOKS
{
    int value;
    char *char_value;
    struct BOOKS *next;
} BOOKS;

BOOKS *insert_value(BOOKS *node, int n, char *char_value);

BOOKS *read_list(BOOKS *head);

int main()
{
    int aux = 0;
    int menu = 0;
    int option;
    BOOKS *head = NULL;
    BOOKS *tail = NULL;

    while (menu != -2)
    {

        int choices;
        printf("1. Insert Book\n");
        printf("2. Print Books\n");
        printf("3. Exit\n");

        scanf("%d", &choices);
        switch (choices)
        {
        case 1:
        {
            int n;
            char char_value[2000] = "";
            printf("Number:");
            scanf("%d", &n);
            printf("Character: ");
            scanf("%s", &char_value);

            if (aux == 0)
            {
                /*FIRST INTERACTION*/
                head = malloc(sizeof(BOOKS));
                tail = malloc(sizeof(BOOKS));
                head = tail = insert_value(tail, n, char_value);
                aux = 1;
            }
            else
            {

                tail->next = malloc(sizeof(BOOKS));
                /*FORMING TAIL->NEXT*/
                tail->next = insert_value(tail->next, n, char_value);
                /*ASSIGNING TAIL->NEXT AS THE NEW TAIL */
                tail = tail->next;
            }
            break;
        }
        case 2:
        { /*READ THE LIST*/
            read_list(head);
            break;
        }
        case 3:
        {

            menu = -2;
            break;
        }

        default:
            printf("Invalid choice\n");
            break;
        }
    }
}

BOOKS *insert_value(BOOKS *node, int n, char *char_value)
{
    node->value = n;
    node->char_value = char_value;
    node->next = NULL;
    return node;
}

BOOKS *read_list(BOOKS *head)
{
    BOOKS *a = head;
    while (a != NULL)
    {

        printf("%d\n", a->value);
        printf("%s\n", a->char_value);
        printf("\n********************\n");
        a = a->next;
    }
}

`node->char_value = char_value;` ... but you don't actually `malloc` and copy.the string. You make `node->char_value` point the `char_value` outside, which is the same `char_value` you use for all inserts. — Ted Lyngmo, May 24 '21 at 15:14
Adding to Ted's comment, in `insert_value`, change `node->char_value = char_value;` into `node->char_value = strdup(char_value);` — Craig Estey, May 24 '21 at 15:18
`node->char_value = char_value` does not copy the string, it only copies the pointer. If you want every node to have its own copy of the string, then you must somehow allocate sufficient space for the string in every node. One way to do this is to use the function `malloc`. You could also make every node contain a fixed number of bytes, but that would probably be wasting a significant amount of memory. — Andreas Wenzel, May 24 '21 at 15:19
Adding to @CraigEstey's comment :-) Don't forget to `free` what you allocate. — Ted Lyngmo, May 24 '21 at 15:19
@CraigEstey strdup is not standard C, I wouldn't recommend using it — ShellCode, May 24 '21 at 15:20
@ShellCode: I agree, but it probably will be in C23, according to [this page](https://en.cppreference.com/w/c/string/byte/strdup). — Andreas Wenzel, May 24 '21 at 15:21
@ShellCode Actually, `strdup` is a POSIX standard. IMO, the source authority for libc is POSIX and _not_ ISO. — Craig Estey, May 24 '21 at 15:27
@CraigEstey POSIX just adds to what is required in libc by the C standard, like ISO/IEC 9899:2018. So you can have a fully compliant C implementation without `strdup`. — Ted Lyngmo, May 24 '21 at 16:00

yano · Accepted Answer · 2021-05-24T15:30:17.483

You're passing char_value from main to your insert_books function, and then saving this pointer in your newly created node. The address of char_value from main doesn't change, so you're saving the same address to each node you create. Since all these pointers point to the same place, they will all read back whatever value was written there last. If you want to make a copy of string in each node, you can use (non standard) strdup or malloc more memory and use strcpy. But, it's an odd design choice you're using a character array to accept one character anyway. I recommend changing char_value in both main and struct BOOKS to char char_value;, then you can simply use equals assignment as with value to save copies of the char. Be sure to scanf a char the correct way.

A couple of other things:

As written, you can change scanf("%s", &char_value); to scanf("%s", char_value);. char_value is a character array, which decays to a char* in this context, there's no need to specify the address of char_value.

head = malloc(sizeof(BOOKS));
tail = malloc(sizeof(BOOKS));
head = tail = insert_value(tail, n, char_value);

This is a memory leak. You're allocating separate memory for head and tail, but then setting both pointers equal to the memory allocated for tail (the return value of insert_value). The memory allocated for head is never freed nor used.

score 0 · Answer 2 · answered May 24 '21 at 15:55

This will help with the errors:-

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

typedef struct BOOKS
{
    int value;
    char char_value[2000];
    struct BOOKS *next;
} BOOKS;

BOOKS *insert_value(BOOKS *node, int n, char *char_value);

BOOKS *read_list(BOOKS *head);

int main()
{
    int aux = 0;
    int menu = 0;
    int option;
    BOOKS *head = NULL;
    BOOKS *tail = NULL;

    while (menu != -2)
    {

        int choices;
        printf("1. Insert Book\n");
        printf("2. Print Books\n");
        printf("3. Exit\n");

        scanf("%d", &choices);
        switch (choices)
        {
        case 1:
        {
            int n;
            char char_value[2000];
            printf("Number:");
            scanf("%d", &n);
            printf("Character: ");
            scanf("%s", char_value);

            if (aux == 0)
            {
                /*FIRST INTERACTION*/
                head = malloc(sizeof(BOOKS));
                tail = malloc(sizeof(BOOKS));
                head = tail = insert_value(tail, n, char_value);
                aux = 1;
            }
            else
            {

                tail->next = malloc(sizeof(BOOKS));
                /*FORMING TAIL->NEXT*/
                tail->next = insert_value(tail->next, n, char_value);
                /*ASSIGNING TAIL->NEXT AS THE NEW TAIL */
                tail = tail->next;
            }
            break;
        }
        case 2:
        { /*READ THE LIST*/
            read_list(head);
            break;
        }
        case 3:
        {

            menu = -2;
            break;
        }

        default:
            printf("Invalid choice\n");
            break;
        }
    }
}

BOOKS *insert_value(BOOKS *node, int n, char *char_value)
{
    node->value = n;
    int size = sizeof(char_value)/sizeof(char);
    strncpy(node->char_value, char_value, size);
    node->next = NULL;
    return node;
}

BOOKS *read_list(BOOKS *head)
{
    BOOKS *a = head;
    while (a != NULL)
    {

        printf("%d\n", a->value);
        printf("%s\n", a->char_value);
        printf("\n********************\n");
        a = a->next;
    }
}

C- Linked List with *chars

2 Answers2