Pointer's value changes without modifying

Question

I'm studying linked list and try to implement some basic function.

My code:

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

#define MAX_LEN_NAME 5

#define INSERT_NODE 1
#define APPEND_NODE 2
#define DEL_HEAD    3
#define DEL_TAIL    4
#define SHOW_LIST   5

struct people {
    int id;
    char name[MAX_LEN_NAME];
    struct people *next;
};

typedef struct people people_list;

static void node_insert(people_list** head_ref, int id, const char* name);
static void node_append(people_list** head_ref, int id, const char* name);
static void node_del_head(people_list** head_ref);
static void node_del_tail(people_list** head_ref);
static void node_show(people_list** head_ref);

static void node_insert(people_list** head_ref, int id, const char* name)
{
    people_list* new_node = NULL;
    new_node = malloc(sizeof(people_list));
    memset(new_node, 0, sizeof(people_list));

    if (new_node == NULL) {
        printf("Fail to allocate memory for new node\n");
        exit(1);
    }

    new_node->id = id;
    memcpy(new_node->name, name, sizeof(name));

    new_node->next = *head_ref;
    *head_ref = new_node;
}

static void node_append(people_list** head_ref, int id, const char* name)
{
    return;
}

static void node_del_head(people_list** head_ref)
{
    return;
}

static void node_del_tail(people_list** head_ref)
{
    return;
}

static void node_show(people_list** head_ref)
{
    people_list* current = NULL;
    current = *head_ref;

    if (current == NULL) {
        printf("Empty list, nothing to show\n");
        return;
    }

    printf("Elements in the list: \n");
    while (current != NULL) {
        printf("id = %d, name = %s\n", current->id, current->name);
        current = current->next;
        if (current == NULL) {
            printf("This is the last element\n");
        }
    }
}

int main(void)
{
    people_list* list = NULL;
#if 0
    people_list* second = NULL;
    people_list* third = NULL;

    list = malloc(sizeof(people_list));
    memset(list, 0, sizeof(people_list));
    second = malloc(sizeof(people_list));
    memset(second, 0, sizeof(people_list));
    third = malloc(sizeof(people_list));
    memset(third, 0, sizeof(people_list));

    list->id = 1;
    memcpy(list->name, "duc", sizeof("duc"));
    list->next = second;

    second->id = 2;
    memcpy(second->name, "hy", sizeof("hy"));
    second->next = third;

    third->id = 3;
    memcpy(third->name, "bo", sizeof("bo"));
    third->next = NULL;
#endif
    char id;
    char name[MAX_LEN_NAME] = {0};
    int option = 0;
    while (1) {
        printf("******************\n");
        printf("1: Insert new node\n");
        printf("2: Append new node\n");
        printf("3: Delete head node\n");
        printf("4: Delete tail node\n");
        printf("5: Show list\n");
        printf("Insert your choice: ");
        scanf("%d", &option);
        switch (option) {
            case INSERT_NODE:
                printf("debug: list=%08Xh\n", list);
                printf("Enter id's value: ");
                scanf("%d", &id);
                printf("Enter name: ");
                scanf("%s", name);
                printf("debug: list=%08Xh\n", list);
                node_insert(&list, id, name);
                memset(name, 0, MAX_LEN_NAME);
                break;
            case APPEND_NODE:
                printf("Enter id's value: ");
                scanf("%d", &id);
                printf("Enter name: ");
                scanf("%s", name);
                node_insert(&list, id, name);
                memset(name, 0, MAX_LEN_NAME);
                break;
            case DEL_HEAD:
                node_del_head(&list);
                break;
            case DEL_TAIL:
                node_del_tail(&list);
                break;
            case SHOW_LIST:
                node_show(&list);
                break;
            default:
                printf ("Invalid input, closing program...\n");
                exit(0);
                break;
        }
    }
}

---

How I test:

• Insert first node (1, jon)
• Insert second node (2, may)
• Show list => Only second node is printed.

I found that, the second->next does not point to the first node, it points to a NULL node. I keep debugging as below

            case INSERT_NODE:
                printf("debug: list=%08Xh\n", list);
                printf("Enter id's value: ");
                scanf("%d", &id);
                printf("Enter name: ");
                scanf("%s", name);
                printf("debug: list=%08Xh\n", list);
                node_insert(&list, id, name);
                memset(name, 0, MAX_LEN_NAME);
                break;

and the console log is

Insert your choice: 1
debug: list=001F29A8h => This is the address holding value of the first node
Enter id's value: 2
Enter name: may  
debug: list=00000000h => The value of list is clear ???

As you can see, the value of the ponter list changes without modifying. Please help me to fix this. Thank you.

Answer 1

As pointed out in the comments, with scanf("%d", &id);, %d is expecting an int *. Change

char id;
char name[MAX_LEN_NAME] = {0};

to

int id;
char name[MAX_LEN_NAME] = {0};

Otherwise you invoke Undefined Behaviour by passing an unexpected type to scanf.

---

With good compiler warnings, we get the following:

warning: 'memcpy' call operates on objects of type 'const char'
while the size is based on a different type 'const char *'
      [-Wsizeof-pointer-memaccess]
        memcpy(new_node->name, name, sizeof(name));

When the sizeof operator is used on a pointer (in this case, a const char *) the result is the size of the pointer in bytes, not the object it points to.

Change this function call to

memcpy(new_node->name, name, sizeof new_node->name);
/* OR memcpy(new_node->name, name, MAX_LEN_NAME); */

or more appropriately, use strcpy

strcpy(new_node->name, name);

---

Suggest changing #define MAX_LEN_NAME 5 to something more reasonable like #define MAX_LEN_NAME 128.

At the same time, scanf("%s", name); should be changed to have a field width specifier that limits the length of the input read, to avoid buffer overflows. This should be the buffer size minus one (e.g., scanf("%127s", name);).

Additionally, the return value of scanf should not be ignored. Ensure it is the expected number of successful conversions, and otherwise handle failure. E.g.,

if (2 != scanf("%d%u", &si, &us)) {
    fprintf(stderr, "Failed to read inputs.\n");
    exit(EXIT_FAILURE);
}

---

Aside: These function calls

people_list* new_node = NULL;
new_node = malloc(sizeof(people_list));
memset(new_node, 0, sizeof(people_list));

can be simplified with calloc

people_list *new_node = calloc(1, sizeof *new_node);