Writing init function for C struct

18,704
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;
}
Share:
18,704
letter Q
Author by

letter Q

Updated on June 04, 2022

Comments

  • letter Q
    letter Q over 1 year

    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?

  • Fiddling Bits
    Fiddling Bits about 10 years
    Don't you have to allocate memory for the strings?
  • simonc
    simonc about 10 years
    @BitFiddlingCodeMonkey Yes. That is already handled by initVariable
  • Admin
    Admin about 10 years
    @BitFiddlingCodeMonkey That depends. In this example, you don't (string literals have static storage duration and they already have storage allocated for them).
  • Admin
    Admin about 10 years
    @simonc It's not even necessary if you are working with string literals. Or who knows? We can't tell unless OP says something about ownership.
  • simonc
    simonc about 10 years
    @H2CO3 I was assuming the question was about whether to allocate memory for strings inside Variable. I agree this wouldn't be necessary for string literals but had thought it better not to limit the solution to this. I'll add a note of a simpler solution for string literals if you think that'd help.
  • Admin
    Admin about 10 years
    @simonc This answer is fine, just saying. (And pointing to OP and looking nasty for not having provided the necessary details...)