Invalid write after allocating memory for struct member in array of structs

Question

I need to pass an array of structs to a function and it is my understanding that I have to allocate memory for the whole array of structs, as well as for each individual struct member in every struct inside the array.

The way I have done it results in an invalid write error from valgrind (caused in the second line inside function read_file). What is wrong?

typedef struct test
{
    char *string1; 
    int num1; 
    int num2;
    char *string2;
} Test;

static void read_file(Test *test)
{
    test = (Test *)calloc(16, sizeof(test));
    test[0].string1 = (char *)calloc(strlen("hello") + 1, sizeof(char));
}

int main(void)
{
    int i = 0;
    Test test[16];

    for (i = 0; i < 16; i++)
    {
        memset(&test[i], 0, sizeof(test[i]));
        test[i] = (Test) { "", 0, 0, "" };
    }

    read_file(test);
    return 0;
}

PS: I know that I have to free the allocated memory, but first I want to get the above code working.

Don't forget that arguments are passed by *value*, i.e. they are *copied*. Modifications (i.e. assignments to the local argument variables) of the copies will not change the original. Think about that when you do the assignment to `test` inside the `read_file` function. — Some programmer dude, Mar 05 '19 at 11:06
Besides, `test` is already pointing to (the first element of) an existing array, why do you need to allocate new memory? — Some programmer dude, Mar 05 '19 at 11:06
Lastly, in C you [should not cast the result of `malloc` (or siblings like `calloc`)](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858). — Some programmer dude, Mar 05 '19 at 11:35

Sander De Dycker · Answer 1 · 2019-03-05T11:12:40.043

2

The test array in main already has memory allocated for it.

You then pass it into the read_file function, so you do not need to allocate memory for it again. Remove this :

test = (Test *)calloc(16, sizeof(test));

Btw, you probably intended sizeof(Test) (or alternatively sizeof(*test)) there. sizeof(test) is the same as sizeof(Test*), which is almost certainly smaller than sizeof(Test).

edited Mar 05 '19 at 11:12

answered Mar 05 '19 at 11:07

Sander De Dycker

16,053
1
35
40

You can also use `sizeof(*test)` instead of `sizeof(Test)`. (Both are valid) – MikeCAT Mar 05 '19 at 11:09
Without that line (test = (Test *)calloc(16, sizeof(test));) the program terminates with the error "Aborted (core dumped)". So I guess I do need it?! – ci7i2en4 Mar 05 '19 at 11:56
@ci7i2en4 : are you sure you're talking about the code you posted in your question ? Or are you doing more than shown ? – Sander De Dycker Mar 05 '19 at 12:02
The snippet in my original posting is part of a much longer code. However, I'm certain that there is no error in parts not shown as I have tested those for weeks without ever encountering any problems. Only since earlier today, after adding what is shown, does the "invalid write" error occur. Or the "Aborted (core dumped)", if I delete the line you mentioned. – ci7i2en4 Mar 05 '19 at 12:06
@ci7i2en4 That suggests your other code has [undefined behavior](https://en.wikipedia.org/wiki/Undefined_behavior) and that it worked by luck. Valgrind should show you where your other code is going wrong. – dbush Mar 05 '19 at 12:14
Even if you are right, Valgrind says the error is in the line test = (Test *)calloc(16, sizeof(Test)); That's why I'm asking here. Without the code snipped I posted, the rest of the code runs perfectly fine and Valgrind doesn't find a single error. – ci7i2en4 Mar 05 '19 at 12:16
^^ edit: I meant this line: test[0].string1 = (char *)calloc(strlen("hello") + 1, sizeof(char)); – ci7i2en4 Mar 05 '19 at 12:25
And without the following line Valgrind says "*** stack smashing detected ***: program terminated": test = (Test *)calloc(16, sizeof(Test)); – ci7i2en4 Mar 05 '19 at 12:26
@ci7i2en4 : the code you posted in your question does not correspond with the behavior you describe. Did you try actually running the code from your question (with my fix), and see if you get the problem there ? – Sander De Dycker Mar 05 '19 at 12:30
I already figured it out; please see my comment to the answer of Kamil Cuk. Apparently, I do need the line test = (Test *)calloc(17, sizeof(Test)); – ci7i2en4 Mar 05 '19 at 12:38
@ci7i2en4 : maybe that's the case in your actual code, but that's *not* the case in the code you posted in your question. In future, to get useful help, make sure that your mcve matches your actual problem. Refer to [How do I ask a good question ?](https://stackoverflow.com/help/how-to-ask) for more details. – Sander De Dycker Mar 05 '19 at 12:41

score 0 · Accepted Answer · answered Mar 05 '19 at 11:20

Test *test

The test variable inside read_file function is a pointer to the Test struct.

sizeof(test)

This is equal to the sizeof of a pointer.

test = (Test *)calloc(16, sizeof(test));

This allocates memory for 16 pointer to Test structure. This does not allocate memory for 16 structures, only for pointers to them.

test[0].string1 =

is invalid and undefined behavior happens. As sizeof(test) is way smaller then sizeof(Test), there is not enough memory to access test0[].string1. So this accesses the memory "out of bounds" and this accesses invalid / not allocated memory region. As you try to write to it (you are doing an assigment) the expresssion is invalid and undefined behavior happens. Valgrind correctly detects that as "write error" - you try to write to memory you don't own.

Even after changing sizeof(test) to sizeof(Test) I still get the same error for test[0].string1 = ... — ci7i2en4, Mar 05 '19 at 11:59
I figured it out: I'm reading a CSV file (into the array of structs) with 16 lines of data. That's the 16 structs I'm (c)allocating memory for. However, there is also the "header line" in the CSV file (describing the columns), which I totally forgot! Now, after correcting this to test = (Test *)calloc(17, sizeof(Test)); my code works perfectly fine. — ci7i2en4, Mar 05 '19 at 12:37

Invalid write after allocating memory for struct member in array of structs

2 Answers2