dynamically allocating strings array

Question

I was just allocating a strings array (2D char array) but it doesn't work very well...

I get this diagnostic from the compiler:

warning: assignment makes integer from pointer without a cast [enabled by default]
error: invalid type argument of unary '*' (have 'int')

Here is the code:

char **a, b, result;
scanf("%d", &couples_n);
a = malloc(couples_n * sizeof(char*));
for (i = 0; i < couples_n; i++) {
    *(a + i) = malloc((MAX_N + 1) * sizeof(char));
}
b = malloc(couples_n * sizeof(char*));
for (i = 0; i < couples_n; i++) {
    *(b + i) = (malloc((MAX_N + 1) * sizeof(char));
}
result = malloc(couples_n * sizeof(char*));
Size = N * 2 + 4;
for (i = 0; i < couples_n; i++) {
    *(result + i) = (char*)malloc(Size * sizeof(char));
}

In [tag:c] declarations are very important, please post them. — Iharob Al Asimi, Jan 18 '16 at 12:17
[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Jan 18 '16 at 12:18
The explanation becomes obvious when more code is provided: `char **a, b, result;` — chqrlie, Jan 18 '16 at 15:15

chqrlie · Answer 1 · 2016-01-18T15:14:12.000

You forgot to include <stdlib.h> where malloc is declared. Note that you only get this warning for the first malloc. The useless cast on the second malloc actually prevents the compiler from diagnosing the suspicious implicit conversion. This is the very reason you should not cast the return value of malloc in C. It is also safer to use the size of the destination type instead of specifying the type explicitly, this way you do not need to update the code if the destination type changes.

Here is a corrected version:

result = malloc(couples_n * sizeof(*result));
Size = N * 2 + 4;
for (i = 0; i < couples_n; i++) {
    result[i] = malloc(Size * sizeof(*result[i]));
}

EDIT

The problem is actually more profound: the definition

char** a, b, result;

Defines a as a pointer to an array of pointers to char, but b and result as simple char variables. It is a bad habit to tack the * to the type: it makes you think that the type is char** whereas in C every variable defined in a single definition can have a different level of indirection. Tack the * to the variable names, and define them correctly.

Change the definition and simplify the code this way:

char **a, **b, **result;

scanf("%d", &couples_n);
a = malloc(couples_n * sizeof(*a));
for (i = 0; i < couples_n; i++) {
    a[i] = malloc((MAX_N + 1) * sizeof(*a[i]));
}
b = malloc(couples_n * sizeof(*b));
for (i = 0; i < couples_n; i++) {
    b[i] = malloc((MAX_N + 1) * sizeof(*b[i]));
}
result = malloc(couples_n * sizeof(*result));
Size = N * 2 + 4;
for (i = 0; i < couples_n; i++) {
    result[i] = malloc(Size * sizeof(*result[i]));
}

Note that you did not post a compilable example or even a complete function: we still don't know how you declare couples_n, i or Size. These definitions might be inconsistent with your usage.

Good guess. I still like to see the declaration of `result`. BTW, this is one of the reasons not to cast `malloc()`. — Iharob Al Asimi, Jan 18 '16 at 12:19
@chqrlie The proper solution to the problem you describe also involves upgrading to a compiler which isn't completely ancient. "Implicit int" was removed from the C language 17 years ago. — Lundin, Jan 18 '16 at 12:27
@stringson Ok then you didn't include stdlib.h. _Please post a minimal example which contains the problem_ so we can reproduce it. Instead of having everyone guessing. — Lundin, Jan 18 '16 at 12:30

dynamically allocating strings array

1 Answers1