Am I initializing a custom struct correctly in C?

Question

So I have the following struct:

struct _Variable {
    char *variableName;
    char *arrayOfElements;
    int32_t address;
};
typedef struct _Variable Variable;

struct _VariableVector {
    int size; // elements full in array
    int capacity; // total available elements
    Variable *variables;
};
typedef struct _VariableVector VariableVector;

Is this the correct way to implement the VariableVector?

VariableVector* initVariableVector() {
    VariableVector* initialVariableVector = malloc(sizeof(VariableVector));
    if (initialVariableVector != NULL ) {
        initialVariableVector->size = 0;
        initialVariableVector->capacity = VECTOR_INITIAL_CAPACITY;
        initialVariableVector->variables = malloc(
                sizeof(Variable) * VECTOR_INITIAL_CAPACITY);
    }
    return initialVariableVector;
}

This is the third question in a row that asks for code review of really basic C programming. Voting to close. — Jens Gustedt, Nov 22 '13 at 13:53

score 0 · Answer 1 · answered Nov 22 '13 at 13:50

0

Your code looks fine, but you are not initialized values in initialVariableVector->variables. Also, try to use calloc() instead of malloc() which fills memory block with zeros. For example:

VariableVector* initialVariableVector = (VariableVector*) calloc (1, sizeof(VariableVector));

answered Nov 22 '13 at 13:50

Artem Agasiev

175
1
8

1

There is no point in using `calloc` when all but one of the elements are being set to non-zero values. – Eric Postpischil Nov 22 '13 at 13:58
What about `initialVariableVector->variables` initialization? Maybe I'm wrong, but I guess that we have to fill all values of `Variable` struct with zeros, to prevent errors in future. – Artem Agasiev Nov 22 '13 at 14:41
`calloc` might or might not be suitable for the allocation to `variables`, depending on what each element of the array needs to be set to when it is put into service. With the code we have, there is no reason to think that setting the contents to zero would necessarily prevent bugs or save time. E.g., if every element of the array up to `size` **must** contain valid pointers (not null) because there is code that dereferences them, then using `calloc` instead of `malloc` just wastes time. – Eric Postpischil Nov 22 '13 at 15:03
Ok, I'm agreed with you, but according to comments at this [question](http://stackoverflow.com/a/1538427/1130967) there is no big difference in call time between `malloc()` and `calloc()`, because OS might already fill memory blocks with zeros. – Artem Agasiev Nov 22 '13 at 15:21
1

When `malloc` or `calloc` must obtain new memory from the operating system, certainly a general consumer-use operating system has an obligation to erase data in the memory it provides, for security purposes, if that memory may contain data the process should not have. However, `malloc` and `calloc` generally reuse previously freed memory before requesting more from the operating system. In these cases, `malloc` does not have any obligation to erase the data. Also, the operating system may have optimizations that allow it to provide unerased memory if it knows it is okay. – Eric Postpischil Nov 22 '13 at 15:26

score 0 · Accepted Answer · answered Nov 22 '13 at 13:57

Instead of returning a pointer to the struct you are trying to initialise it is general practice to pass the pointer to the struct to the initialiser. This will allow you to avoid unnecessary dynamic memory allocations (which are a common source for bugs) and also initialise structs that are in automatic memory or part of other structs:

int initVariableVector(VariableVector* initialVariableVector) {
     // initialise everything to zero first
    memset(initialVariableVector, 0, sizeof(initialVariableVector));

    initialVariableVector->capacity = VECTOR_INITIAL_CAPACITY;

    if (NULL == (initialVariableVector->variables = calloc(
                sizeof(*initialVariableVector->variables) , VECTOR_INITIAL_CAPACITY))
    )
        return -1; // indicate an error

    return 0; // all good
}

Then call it with

VariableVector my_var_vector;
if (initVariableVector(&my_var_vector)) {
    perror("initVariableVector");
    abort();
}

Am I initializing a custom struct correctly in C?

2 Answers2