tags:

views:

282

answers:

4
 char **arr;
 arr = (char **)calloc(1,sizeof(char*));

 for(i = 0; i< 16; i++)
    if(arr[i] = (char *)calloc(1, 2*sizeof(char)) == NULL)
        perror("Memory cannot be allocated to arr[i]", %d);

The above code throws an error inside the for loop, when i am trying to allocate memory to arr[i]. Is anything wrong with this allocation. Essentially, i want to store 16 strings of length 2. I've tried it with array of pointers too (char *arr[16]). I have tried looking for resources on double pointer initializations using malloc() and calloc() and couldn't find many . If you could point out some links, that would be greatly appreciated. Thanks.

+7  A: 

You need to allocate enough memory for 16 pointers, not just one.

arr = (char **)calloc(16, sizeof(char*));

What happens with your code is that arr has enough memory only for one pointer, so arr[0] = <something> is correct, but arr[1] and higher is touching memory that doesn't belong to the program.

Additionally, the way you assign the string pointers is wrong. You are assigning 0 or 1 values, depending on whether the result if calloc is NULL. You need to add parentheses there:

if ((arr[i] = (char *)calloc(1, 2*sizeof(char))) == NULL)
    perror("Memory cannot be allocated to arr[%d]", i);

Er even better:

for(i = 0; i < 16; i++) {
    arr[i] = (char *)calloc(1, 2*sizeof(char));
    if (arr[i] == NULL) {
        perror("Memory cannot be allocated to arr[%d]", i);
    }
}
Lukáš Lalinský
Is that not what i am doing inside the for loop? Am i missing something here.
Deepak Konidena
No, you are allocating memory for one pointer, and then (incorrectly) overwriting memory that doesn't belong to your program.
Lukáš Lalinský
No, you are allocating the individual strings in the array. You must first make sure you have room for 16 pointers to the strings. Try what Lukas suggested and you'll see it works.Also remember you need to free each string + the entire array with 16 individual calls to free()
Isak Savo
I tried the above code. The code compiles fine, but memory is not being allocated to arr[i], perror says memory cannot be allocated.
Deepak Konidena
That's because the assignment is wrong, `a = b == c` is evaluated as `a = (b == c)`, not as `(a = b) == c`. You need to add parentheses there.
Lukáš Lalinský
@Lukáš Lalinský: But that still won't compile. The result of '==' is an int, and the LHS in that assignment is a pointer. You can't assign an int to a pointer without a disgnostic message from the compiler.
AndreyT
@Lukáš Lalinský: Concerning your version of the code - it won't compile. `perror` is not `printf`. It doesn't do what you think it does. `perror` accepts only one parameter.
AndreyT
+1  A: 
arr = (char **)calloc(1,sizeof(char*));

allocates one pointer to pointer to char.

Essentially, i want to store 16 strings of length 2

char **arr = calloc(16, sizeof *arr);

if (!arr) exit(-1); /* bail out somehow */

for(i = 0; i < 16; i++)
  if((arr[i] = calloc(2, sizeof *arr[ i ])) == NULL)
    printf("Memory cannot be allocated to arr[ %d ]", i + 1);

Check the parenthesization as well in your if condition and printf statement. Does your code even compile?

dirkgently
yeah, my code compiles but throws a segmentation fault during runtime.
Deepak Konidena
@Deepak Konidena: Please, don't invent things. The code you posted in the original question does not (and did not) not compile. Either you posted the wrong code, or something else happened, but the code in th OP, once again, is not compilable.
AndreyT
@dirkgently: You forgot `sizeof` in both cases. Also, `perror` doesn't do what you think it does. And why did 16 and 2 (3?) suddenly switch places?
AndreyT
@Andrey - yeah, you're right. I just printed the error string and did not use %d.
Deepak Konidena
Still won't compile. See my comment to the original post.
AndreyT
@AndreyT: Thanks, updated answer.
dirkgently
+3  A: 

When you use calloc, it is customary to use the first parameter to pass the number of elements in the array and the second parameter to pass the size of an element. So, to allocate an array of 16 pointers, one'd normally use calloc(16, <pointer size>), not calloc(1, 16 * <pointer size>), although both do the same thing. In your code you apparently completely forgot about 16 and allocated only 1 pointer.

Don't cast the result of 'calloc'.

Avoid using sizeof(<type>) when calculating size for memory allocation functions. Prefer to use sizeof *<pointer> instead.

If you want to store srings of length 2, you need a buffer of at least 3 characters long (an extra character for zero-terminator).

Memory allocation failure doesn't normally set errno, so perror is not an appropriate function to use here.

Yor assignment to arr[i] in if condition is missing braces. The operations are associated incorrectly. It won't compile as is.

char **arr; 
arr = calloc(16, sizeof *arr); 
for(i = 0; i < 16; i++)
    if((arr[i] = calloc(3, sizeof *arr[i]) == NULL)
        fprintf(stderr, "Memory cannot be allocated");

Finally, an unnamed "magic constant" (16 and 3) is most of the time not a good idea.

AndreyT
Don't cast the result of 'calloc'. Why?
Deepak Konidena
Firstly, it is unnecessary. Secondly, it is fairly dangerous (see the FAQ c-faq.com/malloc/mallocnocast.html). Thirdly, type names are for delarations, they are not supposed to be mentioned in the "regular" code.
AndreyT
But again, the main point is that they are completely *unnecessary*, so the main question here is *why on Earth did you put a cast there*?
AndreyT
People used to C++ usually do that, because it would be an error, or at least a warning, without the cast.
Lukáš Lalinský
AndreyT
Well, I disagree about the 99 out of 100 cases. You can't implicitly cast `void*` in `char*` in C++. At least for me, this is the reason to cast it manually even in C.
Lukáš Lalinský
Well, you must be that 1 out of 100 then :) I, for one, write mostly C++ code, but at the same time have no trouble avoiding excessive casts in C code.
AndreyT
There's a good reason other than custom to prefer `calloc(number, size)` over `calloc(1, number * size)` - in the former, `calloc` should check for the multiplication overflowing the size of `size_t` and wrapping, whereas in the latter you need to do it yourself.
caf
+1  A: 

Storing two characters directly is less expensive than storing a pointer, so I'd suggest dropping one level of indirection and use a contigous block of memory:

char (*arr)[2] = calloc(16, sizeof *arr);

Also keep in mind that your character sequences can't be strings as you didn't provide memory for the terminating 0.

Christoph