Strange behavior of malloc in C
I am trying to create a matrix with dynamic proportions and initialize it, here is the code I use for memory allocation and initialization:
int **matrix;
//mem allocation
matrix=(int*)malloc(sizeof(int*)*mat_w);
for (i=0;i<mat_w;i++)
matrix[i]=(int)malloc(sizeof(int)*mat_h);
//init
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
matrix[i][j]=0;
This works great, the question is, if I try to create a short matrix, I get a segmentation fault on the first init pass.
Is this a C language problem or am I doing something wrong?
Code for matrix type short
:
short **matrix;
//mem allocation
matrix=(short*)malloc(sizeof(short*)*mat_w);
for (i=0;i<mat_w;i++)
matrix[i]=(short)malloc(sizeof(short)*mat_h);
//init
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
matrix[i][j]=0;
PS: I've dropped the security checks, index variables and bounds declarations for clarity of the code.
Thanks,
Alex
source to share
Your casts for the return value malloc()
are not valid. They should be int**
, and int*
in the first case, as short**
and short*
in the second.
When you return the return value malloc()
to short
, the returned pointer is truncated to match the value short
, and then assigned to the pointer short*
, which gives a pointer value that points to invalid memory. Therefore, you get a segmentation fault trying to access it.
Since int
you are lucky, because on your platform, most likely sizeof(int)==sizeof(int*)
, so that the pointer returned malloc()
, discarded on int
, not truncated, and it all works silently. This will most likely work the same on a 64-bit platform.
Should be:
short **matrix;
matrix=(short**)malloc(sizeof(short*)*mat_w);
for (i=0;i<mat_w;i++)
matrix[i]=(short*)malloc(sizeof(short)*mat_h);
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
matrix[i][j]=0;
If your code is pure C (not C ++), you can omit the translations, as in C casting from void*
to any other pointer type is valid.
short **matrix;
matrix = malloc(sizeof(short*)*mat_w);
for (i=0;i<mat_w;i++)
matrix[i] = malloc(sizeof(short)*mat_h);
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
matrix[i][j]=0;
source to share
What compiler are you using to keep it from yelling at you about all these obvious errors?
gcc -Wall
has issued five warning messages with this code.
#include <stdlib.h>
int main ()
{
int mat_w = 99;
int mat_h = 666;
int i;
int j;
int **imatrix;
short **smatrix;
//mem allocation
imatrix=(int*)malloc(sizeof(int*)*mat_w);
for (i=0;i<mat_w;i++)
imatrix[i]=(int)malloc(sizeof(int)*mat_h);
//init
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
imatrix[i][j]=0;
//mem allocation
smatrix=(short*)malloc(sizeof(short*)*mat_w);
for (i=0;i<mat_w;i++)
smatrix[i]=(short)malloc(sizeof(short)*mat_h);
//init
for (i=0;i<mat_w;i++)
for (j=0;j<mat_h;j++)
smatrix[i][j]=0;
return 0;
}
Gives me
malloc.c: In function 'main':
malloc.c:13: warning: assignment from incompatible pointer type
malloc.c:15: warning: assignment makes pointer from integer without a cast
malloc.c:22: warning: assignment from incompatible pointer type
malloc.c:24: warning: cast from pointer to integer of different size
malloc.c:24: warning: assignment makes pointer from integer without a cast
There is a serious lesson you need to learn from this error. And it says this: never return a "malloc" result .
It is also part of a more humane practice with good practice that is best when possible: never mention type names in your code, except in declarations .
This is how your code should have looked from the start
int **matrix;
matrix = malloc(mat_w * sizeof *matrix);
for (i = 0; i < mat_w; i++)
matrix[i] = malloc(mat_h * sizeof *matrix[i]);
for (i = 0; i < mat_w; i++)
for (j = 0; j < mat_h; j++)
matrix[i][j] = 0;
Note that to switch from 'int' to 'short' in this version, you just need to change the 'matrix' declaration and nothing else.
(Of course, there is room for improvement in this code, but I just wanted to address the immediate cause of the error.)
source to share
You are casting int**
into the int*
return value malloc (same for brevity). malloc
should be used like this:
matrix = (int**)malloc(sizeof(int*) * mat_w);
or
matrix = (short**)malloc(sizeof(short*) * mat_w);
The same for every placement inside the matrix:
matrix[i] = (int*)malloc(sizeof(int) * mat_h);
or
matrix[i] = (short*)malloc(sizeof(short) * mat_h);
source to share
Yes, you are doing something wrong.
int *matrix;
means it matrix
is an array of integers. If you want it to be an array of arrays of integers, you must declare it like this:
int **matrix;
//mem allocation
matrix=(int**)malloc(sizeof(int*)*mat_w);
for (i=0; i<mat_w; i++)
matrix[i]=(int*)malloc(sizeof(int)*mat_h);
//init
for (i=0; i<mat_w; i++)
for (j=0; j<mat_h; j++)
matrix[i][j]=0;
Of course, if you know the dimensions of the matrix ahead of time, just do it like this:
int matrix[mat_w][mat_h];
//init
for (i=0; i<mat_w; i++)
for (j=0; j<mat_h; j++)
matrix[i][j]=0;
source to share
sizeof(int)
is equal to the bus width of a particular system. You are trying to fit a 32-bit (or 64 depending on your platform) address value into 16-bit allocated memory.
Check out the second example in the "Checkers" post. This is the correct and preferred way to allocate memory.
source to share