build a linked list but can't carry on the input of the second entry

Question

i wrote a linked list meant to store cpu information. this is a simple input-and-store program and unfortunately get memory leaks. here is the code:

#include<stdio.h>

#include<stdlib.h>

#include<string.h>

#include<math.h>

unsigned input_unsigned_number(void);
double input_double_number(void);
char * gets_s(char ** ptr_to_ptr);
struct cpu_specs {
    /* cpu_specs is a tag of structure */
    char * vendor_id;
    char * cpu_model;
    double cpu_MHz;
    unsigned cache_size: 16; /* set up 16-bit fields */
    unsigned clflush_size: 16; /* set up 16-bit fields */
    struct cpu_specs * next;
};
/* set up the structure declaration */
int main(void) {
    struct cpu_specs * head_to_cpu_stru = NULL;
    struct cpu_specs * previous, * current, * temp;
    char MAX_input[1024];

    fputs("Input the vendor's id(input sole [Enter] to terminate input): ", stdout);
    current = (struct cpu_specs * ) malloc(sizeof(struct cpu_specs));
    /* allocates sizeof(struct cpu_specs) bytes in the midst of uninitialized memory. */
    while (gets_s( & (current - > vendor_id)) != NULL &&
        current - > vendor_id[0] != '\0') {
        if (head_to_cpu_stru == NULL) head_to_cpu_stru = current;
        /* assign the first allocated momery of a pointer to structure cpu_specs to head_to_cpu_stru. */
        else previous - > next = current;
        /* assign the non-first allocated momery of a pointer to structure cpu_specs to previous->next. */
        current - > next = NULL;
        /* set current->next to be NULL. */
        fputs("Input the cpu model(input sole [Enter] to terminate input): ", stdout);
        if (gets_s( & (current - > cpu_model)) == NULL &&
            current - > cpu_model[0] == '\0') break;
        fputs("Input the cpu_MHz(cpu_MHz must be multiplier of 100): ", stdout);
        current - > cpu_MHz = input_double_number();
        fputs("Input the cache_size(cache_size<=65536 and cache_size must be power of 2): ", stdout);
        current - > cache_size = input_unsigned_number();
        fputs("Input the clflush_size(clflush_size<=65536 and clflush_size must be power of 2): ", stdout);
        current - > clflush_size = input_unsigned_number();
        temp = current;
        current = (struct cpu_specs * ) malloc(sizeof(struct cpu_specs));
        /* allocates sizeof(struct cpu_specs) bytes in the midst of uninitialized memory again. */
        previous = temp;
        /* assign the current allocated memory of a pointer to structure cpu_specs to previous. */
    }
    /* gather and store cpu specs from your input. */

    if (head_to_cpu_stru == NULL) puts("No any cpu specs.!");
    else {
        puts("here are all cpu speces:");
        puts("vendor_id\tcpu_model\tcpu_MHz\tcache_size\tclflush_size");
    }
    for (current = head_to_cpu_stru; current - > next != NULL; current = current - > next)
        printf("%s\t%s\t%.2fMHz\t%uKB\t%uKB\n", current - > vendor_id,
            current - > cpu_model, current - > cpu_MHz, current - > cache_size, current - > clflush_size);
    /* show cpu specs in a list */

    for (current = head_to_cpu_stru; current - > next != NULL; current = current - > next)
        free(current);
    puts("program done!");
    return 0;
}

double input_double_number(void) {
    double decimal_number;
    _Bool input_check;
    while ((input_check = fscanf(stdin, "%lf", & decimal_number)) != 1 &&
        fmod(decimal_number, 100.00) == 0) {
        if (input_check != 1) scanf("%*s");
        fprintf(stdout, "invalid input, enter this number again: ");
    }
    return decimal_number;
}

unsigned input_unsigned_number(void) {
    unsigned decimal_number;
    _Bool input_check;
    while ((input_check = fscanf(stdin, "%u", & decimal_number)) != 1 &&
        decimal_number <= 65536 && (decimal_number & decimal_number - 1) == 0) {
        if (input_check != 1) scanf("%*s");
        fprintf(stdout, "invalid input, enter this number again: ");
    }
    return decimal_number;
}

char * gets_s(char ** ptr_to_ptr) {
    unsigned i;
    void * alloc_check;
    char MAX_input[1024];
    if (fgets(MAX_input, 1024, stdin)) {
        for (i = 0; MAX_input[i] != '\n' && MAX_input[i] != '\0' && i < 1024; i++);
        if (MAX_input[i] == '\n') MAX_input[i] = '\0';
        if (( * ptr_to_ptr = (char * ) malloc((strlen(MAX_input) + 1) * sizeof(char))) == NULL)
        /*because the spaces where pointers inside structure point are not allocated and initialized. */
        {
            perror("fail to allocation.\n");
            exit(EXIT_FAILURE);
        }
        strcpy( * ptr_to_ptr, MAX_input);
        return *ptr_to_ptr;
    } else return NULL;
}

input_double_number(), input_decimal_number() are input processings. gets_s() take functionality of initialization memory space where vendor_id, cpu_model points as well as string input.

i could input the first entry in list. when it comes to the second one, it couldn't go further and crash in memory leaks. here is GDB LeakSanitizer:

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7fe5e708dbc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x7fe5e79fe8da in main /home/brushmonk/linkedlist.c:44
    #2 0x7fe5e6c570b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7fe5e708dbc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x7fe5e79fe5ab in main /home/brushmonk/linkedlist.c:23
    #2 0x7fe5e6c570b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

Indirect leak of 9 byte(s) in 2 object(s) allocated from:
    #0 0x7fe5e708dbc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x7fe5e79ff286 in gets_s /home/brushmonk/linkedlist.c:93
    #2 0x7fe5e79fe917 in main /home/brushmonk/linkedlist.c:26
    #3 0x7fe5e6c570b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

Indirect leak of 3 byte(s) in 1 object(s) allocated from:
    #0 0x7fe5e708dbc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x7fe5e79ff286 in gets_s /home/brushmonk/linkedlist.c:93
    #2 0x7fe5e79fe6b2 in main /home/brushmonk/linkedlist.c:35
    #3 0x7fe5e6c570b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: 92 byte(s) leaked in 5 allocation(s).

did i write a wrong gets_s()function and use a pointer to pointer as argument to lead crash?

Can you please check this line:decimal_number <= 65536 && decimal_number & decimal_number - 1 == 0) — Gábor, Jul 20 '20 at 06:10
`while (gets_s( & (current - > vendor_id))` current->vendor_id is an uninitialised pointer. — wildplasser, Jul 21 '20 at 15:03
`memset(current, 0, sizeof(struct cpu_specs));` has initialized all members in structure. @wildplasser — brushmonk, Jul 25 '20 at 14:02
if i delete all `memset(current, 0, sizeof(struct cpu_specs));` statements, how could i initialize every pointer to string in `current`? use `current->vendor_id = (struct cpu_specs * ) malloc(1)`? if there are several pointers to string in structure, how can I initialize every pointer rapidly?@wildplasser — brushmonk, Jul 26 '20 at 04:19
It is close to a FAQ: you are mixing `scanf` to read numeric values and `fgets` to read text. But [scanf() leaves the new line char in the buffer](https://stackoverflow.com/q/5240789/3545273), so for the second item, `fgets` immediately returns the remaining newline, while you do not expect it. IMHO: time to learn how to use a debugger... — Serge Ballesta, Jul 26 '20 at 15:30

Ilian Zapryanov · Accepted Answer · 2020-07-26T15:29:45.417

1

I've debugged your code and changed it as follows:

In Line 30 you alloc a string but never free it: my change:

char* getres = NULL;
while ((getres = gets_s( & (current->vendor_id))) != NULL &&
       current->vendor_id[0] != '\0') {
    free(getres);

Finally (I've intentionally remained your loop but commented), you did not walk in the list correctly and not free anything. Also have 3 pointers not freed.

struct cpu_specs* it = head_to_cpu_stru;
while (it != NULL) {
    struct cpu_specs* tmp = it;
    it = it->next;
    free(tmp);
}
// your loop:
//    for (current = head_to_cpu_stru; current != NULL; current = current->next)
//        free(current);

See compared to your commented loop. Also use valgrind for memchecks if on unix-like OS. But IMHO you have other problems in code design as reaollicating to a same pointer and potential dangling pointers, pointers not set to NULL, etc., but I keep to the question you asked about the leak.

edited Jul 26 '20 at 15:29

answered Jul 26 '20 at 15:13

Ilian Zapryanov

1,132
2
16
28

in Line 30 I use return value of `gets_s()` as every loop condition. But i didn't use return value of `gets_s()` in other place other than loop condition. why did it leak? @Ilian Zapryanov – brushmonk Jul 27 '20 at 08:42
your `gets_s()` function does return pointer to allocated data and also assigns to a pointer. Why just not return and check data, rather than have multiple return statements and enter error prone functions? Who will delete your structs assigned pointers? Also `strndup` also allocates and duplicates a string. `get_s` can return `bool` on succeeded/failed `malloc` rather pointers. – Ilian Zapryanov Jul 27 '20 at 11:03
You `malloc` in `gets_s()`, then return in the `if` expression, check for null then nothing points to that memory. It's dangling, can't be reached anymore. It's `malloc` so it's heap of course. You have to point to those memories in order to free them. Remove my assignment and `free` to the `if` code and will see lost blocks of memory. – Ilian Zapryanov Jul 27 '20 at 13:44

build a linked list but can't carry on the input of the second entry

1 Answers1