Reputation: 27
My problem is prettty simple, I wanna allocate memory for a 2d array in c, fill it with -1, then free it and exit the program. My code keeps crashing and I dont know what I am doing wrong... This is what I got:
int main(){
int i,j;
char str1[]="xxxabxcxxxaabbcc";
char str2[]="abc";
int len1=strlen(str1);
int len2=strlen(str2);
printf("%d %d",len1,len2);
//allocate 2d_array
int **H_table = (int**)malloc((len1+1)*sizeof(int));
for (i=0; i<len1+1; i++){
H_table[i] = (int*)malloc((len2+1)*sizeof(int));
}
//fill and print 2d array
for(i=0;i<len1+1;i++){
for(j=0;j<len2+1;j++){
printf("i:%d j:%d",i,j);
H_table[i][j]=-1;
printf(" value:%d\n",H_table[i][j]);
}
}
// free 2d array
for(i=0;i<len1;i++){
free(H_table[i]);
}
free(H_table);
return 0;
}
So what happens is that I wanna allocate an array that has 1 extra line and 1 extra column than the 2 strings if you put them vertically compared to each other.
And this is what I expected( the things in bracets are obviously not part of the table, I put the there for comparison):
(x x x a b x c x x x a a b b c c)
1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
a)1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
b)1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
c)1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
The problem is that the code crashes when it fills the table, and it always crashes for i=9 and j=3, for those specific strings. The weird part is that if you swap the 2 strings(put "abc" in str1) then the code passes the filling stage, and crashes when it tries to free the array.
Sorry for any grammar mistakes or stackoverflow mistakes, I am kinda new here :P
Any idea is welcome :) thx in advance
Upvotes: 0
Views: 49
Reputation: 241971
As a number of people have pointed out, you're allocating H_table
with room for len1 + 1
integers but it is actually supposed to be an array of len1 + 1
pointers (to integers). Since pointers are bigger than integers (on your system, anyway), you end up with undefined behaviour from a buffer overrun.
Here's a hint. Avoid this problem, and a variety of other similar issues, by always using the following model for malloc
:
some_variable = malloc(n * sizeof *some_variable);
For example:
int** H_table = malloc((len1 + 1) * sizeof *H_table);
for (int i = 0; i <= len1; ++i)
H_table[i] = malloc((len2 + 1) * sizeof *H_table[i]);
That is, let the compiler figure out the right type for the variable (or lvalue). The compiler is less prone to typos than you are, and not writing the type explicitly will make it a lot easier for you to later decide that H_table
should have been long
or short
or unsigned
.
For the same reason, don't explicitly cast the return value of malloc
. C automatically casts void*
to the destination type, and does not provide an error if you manually cast to the wrong type. So just let the compiler do it; it's less typing, safer, and more future-proof.
Note that if you use an expression with sizeof
, the compiler does not evaluate the expression [Note 1]. It just figures out the type and substitutes that for the expression. So don't worry about extra evaluation: there isn't any. That's also why it's ok to use this model with declarations, even though some_variable
doesn't yet have a value when the malloc
is executed.
ex
in sizeof ex
: if ex
is a variable-length array. However, in this case ex
is always a pointer, so that case cannot apply.Upvotes: 1
Reputation: 49
As @xing mentioned in his comment, H_table
is a pointer to pointer to integer. so you need to change the int
to int*
in the first malloc
.
here:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(){
int i,j;
char str1[]="xxxabxcxxxaabbcc";
char str2[]="abc";
int len1=strlen(str1);
int len2=strlen(str2);
printf("%d %d",len1,len2);
//allocate 2d_array
int **H_table = (int**)malloc((len1+1)*sizeof(int*));
for (i=0; i<len1+1; i++){
H_table[i] = (int*)malloc((len2+1)*sizeof(int));
}
//fill and print 2d array
for(i=0;i<len1+1;i++){
for(j=0;j<len2+1;j++){
printf("i:%d j:%d",i,j);
H_table[i][j]=-1;
printf(" value:%d\n",H_table[i][j]);
}
}
// free 2d array
for(i=0;i<len1;i++){
free(H_table[i]);
}
free(H_table);
return 0;
}
Upvotes: 0