How to correctly dynamically allocate memory?

Question

The code below was made from examples taken from this site. I can't understand, what am I doing wrong? Could you please help me out?

Compiling with:

gcc -std=c11 main.c

Prints only:

Thing: Boiled buckwheat, weight: 1500

Segmentation fault

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

typedef struct {
    // Weight in grams
    size_t weight;
    // Name of Thing
    char name[255];

} Things;

void add_new_thing(Things **things,size_t *size)
{

    size_t index = *size;

    if(index == 0){
        (*size) = 1;
        *things = (Things*)calloc((*size),sizeof(Things));
        if (*things == NULL) {
            fprintf(stderr, "Error: can't allocate memory! %s\n", strerror(errno));
            exit(EXIT_FAILURE);
        }
    }else{
        (*size) += 1;
        Things *temp = (Things*)realloc(*things,(*size)*sizeof(Things));
        if(temp != NULL) {
            *things = temp;
        }else{
            fprintf(stderr, "Error: can't reallocate memory! %s\n", strerror(errno));
            exit(EXIT_FAILURE);
        }
        // Zeroing of new structure's elements
        things[index]->name[0] = '\0';
        things[index]->weight = 0;
    }

}

void another_function(Things *things,size_t *size)
{
    // Add one element to the array of structures
    add_new_thing(&things,size);
    const char *str1 = "Boiled buckwheat";
    strncpy(things[*size-1].name, str1, strlen(str1) + 1);
    things[*size-1].weight = 1500;

    for(size_t i = 0;i < *size;i++){
        printf("Thing: %s, weight: %zu\n",things[i].name,things[i].weight);
    }

    // Add one more element to the array of structures
    add_new_thing(&things,size);
    const char *str2 = "A toy";
    strncpy(things[*size-1].name, str2, strlen(str2) + 1);
    things[*size-1].weight = 350;

    // Segmentation fault is below
    for(size_t i = 0;i < *size;i++){
        printf("Thing: %s, weight: %zu\n",things[i].name,things[i].weight);
    }
}

void some_function(Things *things,size_t *size)
{
    // To pass the array of structures to another function
    another_function(things,size);
}

int main(void)
{

    // Create NULL pointer for the array of structures
    Things *things = NULL;

    // Add size of structures' array which will be allocated within add_new_thing() function
    size_t size = 0;

    // Call some function
    some_function(things,&size);

    // Segmentation fault is below
    printf("Print results:\n");
    for(size_t i = 0;i < size;i++){
        printf("Thing: %s, weight: %zu\n",things[i].name,things[i].weight);
    }

    free(things);

    return(EXIT_SUCCESS);
}

what is the essence of `some_function` if you only use it to call another function. why not pass the address of `things` straight to another function — Seek Addo, Jun 13 '17 at 14:14
https://stackoverflow.com/questions/2590931/how-to-debug-a-c-program — Stargateur, Jun 13 '17 at 14:14
would make more sense for your `add_new_thing` function to also take in values for the new "thing" you're creating — Chris Turner, Jun 13 '17 at 14:15
My detailed example had taken from real program and have structure of the functions like real. It is very important because more simples examples works without errors. — Dennis V, Jun 13 '17 at 14:18
your code is terrible,thing about your `things` pointer didn't changed. — K.Juce, Jun 13 '17 at 14:19

score 4 · Answer 1 · answered Jun 13 '17 at 14:15

Remember that C have call by value, which means in the main function you're passing a copy of the null-pointer in things to some_function. The actual variable in main will not change.

Only in the another_function do you emulate pass by reference, and only in another_function will the things variable have been updated by the allocations in add_new_thing.

Iharob Al Asimi · Accepted Answer · 2017-06-13T14:25:43.537

The real problem is here

// Zeroing of new structure's elements
things[index]->name[0] = '\0';
things[index]->weight = 0;

It MUST be

(*things)[index].name[0] = '\0';
(*things)[index].weight = 0;

It is because, things is not a pointer of pointers, but simply a pointer.

You are treating things as if it was a pointer to an array of pointers, but it's just a pointer to an "array" of Things.I say "array", because it's not strictly speaking an array, arrays are a different thing in c. But it's for all purposes the same as an array.

You also, create the pointer in main, but you never use that copy of the pointer correctly, you still free() it.

Try to read the corrected code, and see if you can understand your mistake

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

typedef struct
{
    // Weight in grams
    size_t weight;
    // Name of Thing
    char name[255];

} Things;

void add_new_thing(Things **things,size_t *size)
{

    size_t index = *size;

    if(index == 0)
        {
            (*size) = 1;
            *things = (Things*)calloc((*size),sizeof(Things));
            if (*things == NULL)
                {
                    fprintf(stderr, "Error: can't allocate memory! %s\n", strerror(errno));
                    exit(EXIT_FAILURE);
                }
        }
    else
        {
            (*size) += 1;
            Things *temp = (Things*)realloc(*things,(*size)*sizeof(Things));
            if(temp != NULL)
                {
                    *things = temp;
                }
            else
                {
                    fprintf(stderr, "Error: can't reallocate memory! %s\n", strerror(errno));
                    exit(EXIT_FAILURE);
                }
            // Zeroing of new structure's elements
            (*things)[index].name[0] = '\0';
            (*things)[index].weight = 0;
        }

}

void another_function(Things **things, size_t *size)
{
    // Add one element to array of structures
    add_new_thing(things,size);
    const char *str1 = "Boiled buckwheat";
    strncpy((*things)[*size-1].name, str1, strlen(str1) + 1);
    (*things)[*size-1].weight = 1500;

    for(size_t i = 0; i < *size; i++)
        {
            printf("Thing: %s, weight: %zu\n",(*things)[i].name,(*things)[i].weight);
        }
    // One element of array of structures was printed there

    // Add new one element to array of structures
    add_new_thing(things, size);
    const char *str2 = "A toy";
    strncpy((*things)[*size-1].name, str2, strlen(str2) + 1);
    (*things)[*size-1].weight = 350;

    // Segmentation fault is there
    for(size_t i = 0; i < *size; i++)
        {
            printf("Thing: %s, weight: %zu\n",(*things)[i].name,(*things)[i].weight);
        }
}

void some_function(Things **things, size_t *size)
{
    // Pass array of structures to another function
    another_function(things, size);
}

int main(void)
{

    // Create NULL pointer for array of structures
    Things *things = NULL;

    // And size of structures array which will be allocated within add_new_thing() function
    size_t size = 0;

    // Call some function
    some_function(&things, &size);

    // Segmentation fault is there
    printf("Print results:\n");
    for(size_t i = 0; i < size; i++)
        {
            printf("Thing: %s, weight: %zu\n",things[i].name,things[i].weight);
        }
    free(things);

    return(EXIT_SUCCESS);
}

Thank you very much for detailed explanation. I'm newbie in C. I have learning it just three years. ) — Dennis V, Jun 13 '17 at 14:34

score 1 · Answer 3 · answered Jun 13 '17 at 14:17

In your main function you are passing the value of things (which is NULL) to the function some_function(). So this pointer is not changed, you need to pass it's address. The printf() call tries to access to what is store at NULL. (obviously this is not possible)

How to correctly dynamically allocate memory?

3 Answers3