Dynamic memory allocation for array passed as reference in C recursive function

Question

I'd like to count how much my recursion is nested and return every level number in an array created inside function, but that code doesn't work properly (it displays "Segmentation fault" when ran):

void test (int* result[], int* counter) {


  if (*counter == 0)
        *result = (int**) malloc(0);

    if (*counter == 5)
        return;

    else {
        *counter = *counter + 1;
        *result = (int**) realloc(*result, sizeof(int) * (*counter));
        *result[*counter - 1] = *counter; // This line is wrong, and I don't know why.
        test(result, counter);
    }
}

void main () {
    int** result;
    int counter = 0;

    test(result, &counter);

    int i;
    for (i = 0; i < 5; i++)
        printf("%d\n", *result[i]);
}

I know I could do it using single pointer as parameter and as return value. The point is, I'm just curious.

"Doesn't work properly" is not a useful description. Please explain what behaviour you're observing, and what you expect. — Oliver Charlesworth, Apr 13 '14 at 16:46
Also, do not add superfluous casts (like for the return of `malloc`). Only cast where neccessary for a correct program, never to shut up the compiler. Ask the compiler for all warnings using `-Wall -Wextra`, and handle them appropriately. `*result` is not an `int**` btw. — Deduplicator, Apr 13 '14 at 16:55
For your own good, try to compile your code with `-Wall` as a habit: it would show you all your small mistakes here and there; such as the use of `int**` as a cast for an `int*` value; the lack of an integer return for `main` (and the wrong prototyping for `main` also), or the initialized `result` variable. About that one, it would be worth it to `malloc` it, since it can randomly segfault in the current state of your code; and correcting it as `-Wall` would suggest (e.g. having `int** result = NULL;`) will show you that it segfaults, but NOT randomly, so you can correct it ;) — 7heo.tk, Apr 13 '14 at 18:48
Using `malloc(0)` even by accident is a bad idea; doing it by writing it explicitly is a very bad idea. You can't dereference whatever pointer is returned reliably. — Jonathan Leffler, Apr 14 '14 at 21:19

score 3 · Answer 1 · edited May 23 '17 at 12:13

3

The code for test() looks superficially ok to me (I didn't test it, so I can't say for sure), but this is wrong in main():

for (i = 0; i < 5; i++)
    printf("%d\n", *result[i]);
}

You want (*result)[i]. Array indexing has higher priority than pointer dereferencing.

You also shouldn't cast malloc()'s return value (see Do I cast the result of malloc?), especially since you did it wrong: *result is of type int *, so if you want to keep the cast, at least change it to int *, but I would recommend taking it off entirely.

UPDATE:

The same happens for this line:

*result[*counter - 1] = *counter;

It should be:

(*result)[*counter - 1] = *counter;

edited May 23 '17 at 12:13

Community

1
1

answered Apr 13 '14 at 16:57

Filipe Gonçalves

20,783
6
53
70

+1 for pointing out the priority of the array indexing operator over the pointer dereferencing. But it also is the reason why the third line of the else block of the test function fails. That would be worth mentioning, since it's the question OP asked... – 7heo.tk Apr 13 '14 at 18:22
@7heo.tk Oh, right. Didn't catch that one, thanks for noting. Updated my answer. – Filipe Gonçalves Apr 13 '14 at 18:38
Anytime ;) - Now the code of OP can compile and do what (s)he wants. – 7heo.tk Apr 13 '14 at 18:41

Deduplicator · Answer 2 · 2014-04-13T18:50:47.227

A rundown of all questionable or wrong lines with reasons:

*result = (int**) malloc(0);

That line either sets *result to 0, or to some unique value which must be freed. Also, the cast is superfluous and wrong, as *result has type int*.

*counter = *counter + 1;

The above yells for the standard increment operator: ++*counter;

*result = (int**) realloc(*result, sizeof(int) * (*counter));

In addition to the type and cast issues identified with malloc above, better write it this way to avoid using the wrong size: *result = realloc(*result,*counter * sizeof *result);

*result[*counter - 1] = *counter; // This line is wrong, and I don't know why.

Operator precedence means you need to add parentheses: (*result)[*counter - 1] = *counter;

void main () {

The only blessed prototypes of main are int main(void) and int main(int argc, char* argv[argc]) or compatible. Implementations may allow others too, though they are non-portable.

int** result;

x You want a int* here, because you want to use it as a reference argument to test.

test(result, &counter);

Comment x means you need to take the address of result above.

for (i = 0; i < 5; i++)

You might consider using *counter as the limit for genericity, though in your case test really always sets it to 5 in the end.

printf("%d\n", *result[i]);

Remove the dereferencing for good results, as comment x says.

`*counter++` is not the same as `*counter = *counter+1;`. The equivalent form is `(*counter)++`, or `++*counter` (stricly speaking, the second one *is* the only equivalent statement). You want to increment the pointed-to object, not the pointer. And yes, there is something wrong with the line `*result[*counter - 1] = *counter;` - again, a priority issue: it should be `(*result)[*counter - 1] = *counter;`, because the dynamic array lives in `*result`. — Filipe Gonçalves, Apr 13 '14 at 18:44
@FilipeGonçalves I hate running afool of operator precedence. Good catch. — Deduplicator, Apr 13 '14 at 18:46

Dynamic memory allocation for array passed as reference in C recursive function

2 Answers2