Writing init function for C struct

Question

So this is my struct in a header file:

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

and here is my implementation of the init function in .c file:

void initVariable(Variable *variable, char *variableName, char *arrayOfElements,
        int32_t address) {
    int lengthOfVariableNameWithTerminatingChar = strlen(variableName) + 1;
    variable->variableName = malloc(
            sizeof(char) * lengthOfVariableNameWithTerminatingChar);
    strncpy(variable->variableName, variableName,
            lengthOfVariableNameWithTerminatingChar);

    int lengthOfArrayOfElementsWithTerminatingChar = strlen(arrayOfElements)
            + 1;
    variable->arrayOfElements = malloc(
            sizeof(char) * lengthOfArrayOfElementsWithTerminatingChar);
    strncpy(variable->arrayOfElements, arrayOfElements,
                lengthOfArrayOfElementsWithTerminatingChar);

    variable->address = address;
}

I get no errors when I compile but when I run my test file:

void test_initVariable() {
    printf("\n---------------test_initVariable()-----------------\n");
    // TODO:
    Variable *variable1;
    initVariable(variable1, "variable1", "1, 2, 3", 4); // <== Causes binary .exe file to not work
}

Can anyone tell me how to fix my implementation?

Answer 1

Variable *variable1;

gives you an uninitialised pointer. You don't own the memory it points to so can't safely write to it.

You need to allocate storage for variable1

Variable variable1;
initVariable(&variable1, "variable1", "1, 2, 3", 4);

would work.

If you want variable1 to be dynamically allocated, it'd be easiest to have initVariable handle this

Variable* initVariable(char *variableName, char *arrayOfElements, int32_t address)
{
    Variable* var = malloc(sizeof(*var));
    if (var != NULL) {
        var->variableName = strdup(variableName);
        var->arrayOfElements = strdup(arrayOfElements);
        var->address = address;
    }
    return var;
}

Note that I've also simplified allocation/population of strings here. Your code works but if you're using a posix-compatible system, strdup is a much simpler way to achieve the same results.

As discussed in comments, you don't need to allocate storage if the string members of Variable will all be string literals. In this case, you could simplify things to

Variable* initVariable(char *variableName, char *arrayOfElements, int32_t address)
{
    Variable* var = malloc(sizeof(*var));
    if (var != NULL) {
        var->variableName = variableName;
        var->arrayOfElements = arrayOfElements;
        var->address = address;
    }
    return var;
}