program crashes at repetitive calloc() call

Question

Edit: solved by kaylums little comment. Thank you!

good morning, I am relatively new to C still and I'm trying to make a doubly linked list. I got my program to run properly with all the functions with this kind of element:

the program crashes after either 2 or 3 inserted elements in the list in the calloc() call of my insertElement() function. I don't get any SIGSEGV or anything, the program just stops with a random negative return. I'll try to give a minimum code example of the function and the function call:

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

typedef struct Element {
    char name[30];
}Element;

typedef struct List {
    int size;
    Element* first;
    Element* last;
}List;



Element* insertElement(List* List, char name[30]) {
    Element* element;
    element = (Element*)calloc(0, sizeof(Element));
    strncpy_s(element->name, name, 30);
    return element;
}

List globalList;
char name[30];

int main() {
    while (true) {
        printf("insert the name >>");
        if (fgets(name, 30, stdin) != NULL)
            name[strcspn(name, "\n")] = 0;
        insertElement(&globalList, name);
    }
}

is there already something obvious wrong with that basic stuff? Thank you very much in advance! Any advice would be very much appreciated, have a good day!

Answer 1

I will not extend on the actual problem (specifying 0 as the number of elements requested to calloc()). I will point you to several other things found in your code.

The first problem in reading your code is that you lack to include the file <stdbool.h>, necessary to use the constants true and false and the type bool. I have added it in the first line.

#include <stdbool.h>

Next, you use at several places the value 30 as the size of several objects that are all related. If you decide in the future to change that value, it will be difficult to find all the ocurrences of the constan 30 and change all of them (and the risk you have used also 30 for anything else and it gets changed in the middle)

I have included a constan with the following lines:

#define NAME_LENGTH  (30)

and all the definitions: ...

    char name[NAME_LENGTH];

in the structure...

Element* insertElement(List* List, char name[NAME_LENGTH]) {

in the prototype of insertElement (you don't need as name is actually defined as char *, not as an array of NAME_LENGTH elements...

On other side, you need to include a pointer on each Element to link each to the next element of the list. This is done right after name:

    struct Element *next; /* we need to include struct as the type Element is not yet defined */

Next, include sizeof *element as the second parameter to calloc() and 1 to the first. Better, if you are going to initialize all fields in the Element structure, then it is better to call malloc() (see the final code , posted at the end)

NEVER, NEVER, NEVER cast the value returned by malloc() (and friends) This is a legacy that causes a lot of errors, that get undetected (and very difficult to find), due to the cast. When you cast you tell the compiler: leave it in my hands, as I know what I'm doing. And this makes the compiler silent, when it should be complaining. The problem mainly has to do with forgetting to include the header file where malloc (and friends) are declared (<stdlib.h>) and you will take long time to detect and see why your program has crashed.

For the same reason, don't use the size of the type, when you can use the pointed to expression as template of the type. This is because if you change the type of the pointed to object, you need to remember that here you have put the type of the object (and you need to change it too) This way, this expression will only be bad if you change the object into a non pointer object. Also, you have requested for 0 elements of the specified type, which has already been noticed in other answers. This will make calloc() to return NULL, value you don't check in your code, and you try to use it later on. This will crash your program, but in the best case, it is Undefined Behaviour (and a very difficult error to find, so be careful and always check the value returned by malloc()).

Next, don't use strncpy_s() as it is Microsoft specific routine, and isn't included in any standard. A proper substitute has been provided by strncpy():

    strncpy(element->name, name, sizeof element->name);

also use the sizeof operator, as it protects you if you decide in the future to change the type of the pointer.

Finally, it is better to use fgets() as the test expression for the while statement in main(). The reason is that you can end the loop when the end of file is detected.

Finally, you code ends as (including the linking of Elements in the linked list):

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

#define NAME_LENGTH     (30)

typedef struct Element {
    char name[NAME_LENGTH];
    struct Element *next;
} Element;

typedef struct List {
    int size;
    Element* first;
    Element* last;
} List;



Element* insertElement(List* List, char name[NAME_LENGTH]) {
    Element* element;
    /* NEVER, NEVER, NEVER cast the value returned by malloc
     * (and friends) This is a legacy that causes a lot of
     * errors, that get undetected (and very difficult to find),
     * due to the cast.  When you cast you tell the compiler:
     * leave it in my hands, as I know what I'm doing.  And this
     * makes the compiler silent, when it should be complaining.
     * The problem mainly has to do with forgetting to include
     * the header file where malloc (and friends) are declared
     * (<stdlib.h>)  and you will take long time to detect and
     * see why your program has crashed. */
    /* for the same reason, don't use the size of the type, when
     * you can use the pointed to expression as template of the
     * type.  This is because if you change the type of the
     * pointed to object, you need to remember that here you have
     * put the type of the object.  This way, this expression
     * will only be bad if you change the object into a non
     * pointer object.  Also, you have requested for 0 elements
     * of the specified type. */
    element = malloc(sizeof *element);
    /* don't use strncpy_s as it is not standard. Use the sizeof
     * operator again, to protect the expression if you change
     * the type of element->name */
    strncpy(element->name, name, sizeof element->name);
    element->next = NULL;
    if (List->last) {
        List->last->next = element;
        List->last = element;
    } else {
        List->first = List->last = element;
    }
    return element;
}

List globalList;
char name[NAME_LENGTH];

int main() {
    /* if you put the fgets() call as the test of the while
     * statement below, you will process each line until you get
     * an end of file condition. Then you can do both things: to
     * null the occurence of the \n char, and the call to
     * insertElement()  I have not corrected because it's a
     * question of taste. */
    printf("insert the name >> ");
    while (fgets(name, sizeof name, stdin) != NULL) {
        /* sizeof name is better than the constant, as if you
         * change the type definition of object name, you have to
         * remember that you are using here its size.  sizeof
         * does the job for you. */
        name[strcspn(name, "\n")] = 0;
        insertElement(&globalList, name);
        printf("insert the name >> ");
    }
    Element *p;
    char *sep = "\n\n{ ";
    for (p = globalList.first; p; p = p->next) {
        printf("%s\"%s\"", sep, p->name);
        sep = ", ";
    }
    printf(" };\n");
}

Answer 2

element = (Element*)calloc(0, sizeof(Element));

what is 0 in first argument?
actually you ask for 0 number of your type from memory!

here is some explanation about dynamic memory allocation:
Dynamic memory allocation is a process of allocating memory at run time. There are four library routines, calloc(), free(), realloc(), and malloc() which can be used to allocate memory and free it up during the program execution. These routines are defined in the header file called stdlib.h.

What is malloc() ?

It is a function which is used to allocate a block of memory dynamically. It reserves memory space of specified size and returns the null pointer pointing to the memory location.

The pointer returned is usually of type void. It means that we can assign malloc function to any pointer. The full form of malloc is memory allocation.

What is calloc() ?

Calloc() function is used to allocate multiple blocks of memory. It is a dynamic memory allocation function which is used to allocate the memory to complex data structures such as arrays and structures. If this function fails to allocate enough space as specified, it returns will null pointer. The full form of calloc function is contiguous allocation.

Why use malloc() ?

Here are the reasons of using malloc()

You should use malloc() when you have to allocate memory at runtime.
You should use malloc when you have to allocate objects which must exist beyond the execution of the current memory block.
Go for malloc() if you need to allocate memory greater than the size of that stack.
It returns the pointer to the first byte of allocated space.
It enables developers to allocate memory as it is needed in the exact amount.
This function allocates a memory block size of bytes from the heap.

Why use calloc() ?

Here are the reasons of using calloc()

When you have to set allocated memory to zero.
You can use calloc that returns a pointer to get access to memory heap.
Used when you need to initialize the elements to zero to returns a pointer to the memory.
To prevent overflow that is possible with malloc()
Use calloc() to request a page that is known to already be zeroed.

Syntax of malloc() Here is a Syntax of malloc()

ptr = (cast_type *) malloc (byte_size);

n above syntax, ptr is a pointer of cast_type. The malloc function returns a pointer to the allocated memory of byte_size.

Example of malloc() in C
In the bellow code, sizeof(*ptr) is used to allocate a memory block of 15 integers. In the printf statement, we are finding the value of the 6th integer.

#include<stdlib.h>
#include<stdio.h>
int main(){
int *ptr;
ptr = malloc(15 * sizeof(*ptr)); 
    if (ptr != NULL) {
      *(ptr + 5) = 480; 
      printf("Value of the 6th integer is %d",*(ptr + 5));
    }
}

Output:

Value of the 6th integer is 480

Syntax of calloc()
Here is a Syntax of malloc()

ptr = (cast_type *) calloc (n, size);

The above syntax is used to allocate n memory blocks of the same size. After the memory space is allocated, all the bytes are initialized to zero. The pointer, which is currently at the first byte of the allocated memory space, is returned.

Example of calloc() in C
The C language program below calculates the sum of the first ten terms. If the pointer value if null, then the memory space will not be allocated. For loop is used to iterate the value of a variable "i" and print the sum. Lastly, function free is used to free-up the pointer.

#include <stdio.h>
#include <stdlib.h>
    int main() {
        int i, * ptr, sum = 0;
        ptr = calloc(10, sizeof(int));
        if (ptr == NULL) {
            printf("Error! memory not allocated.");
            exit(0);
        }
        printf("Building and calculating the sequence sum of the first 10 terms \n");
        for (i = 0; i < 10; ++i) { * (ptr + i) = i;
            sum += * (ptr + i);
        }
        printf("Sum = %d", sum);
        free(ptr);
        return 0;
    }

Output:

Building and calculating the sequence sum of the first 10 terms n Sum = 45