views:

191

answers:

7

I have a problem about malloc(). It is weird. My code is in the following. I use random generator to generate elements for an array. The array is opened by malloc(). If the array size is smaller than 8192, it is OK. If the size is larger than 8192, it shows segment fault.

void random_generator(int num, int * array) {

  srand((unsigned)time(0)); 
  int random_integer; 
  for(int index=0; index< num; index++){ 
    random_integer = (rand()%10000)+1; 
    *(array+index) = random_integer; 
    cout << index << endl;
  } 
}

int main() {
  int array_size = 10000;
  int *input_array;
  input_array = (int*) malloc((array_size));
  random_generator(8192, input_array);    // if the number is larger than 8192, segment fault
  free(input_array);
}
+10  A: 

You want:

input_array = (int*) malloc( array_size * sizeof(int) );

you might consider the much simpler:

input_array = new int[ array_size ];
// stuff
delete [] input_array;

or even:

std::vector <int> input_array( array_size );

and not have to worry about calling free or delete, or about exceptions.

anon
Interesting downvote there - which of the above do you think is technically incorrect?
anon
Maybe they thought you were a bit too radical with your ultra-modern C++ features... `</sarcasm>`
Skilldrick
@Skilldrick Nah - it just looks like one of the clueless is off on another pointless downvoting spree. Hint to the clueless - the SO admin scripts detect this kind of stuff.
anon
+13  A: 

malloc() takes the size in bytes, not the number of elements. The size of an int is typcially 4 bytes, so you are actually allocating only enough memory for 2500 integers. You are allocating array_size bytes, while you should be allocating array_size * sizeof(int) bytes.

So, the error will be fixed by

input_array = (int*) malloc(array_size * sizeof(int));

P.S. Never assume that you know the size of an int or any other data type, as it is platform dependent. Always use sizeof().

P.P.S. This is really a C question, rather than a C++ question. If you are actually using C++, you should consider using new and delete [] instead of malloc() and free(), or better yet use std::vector instead of an array, as Neil pointed out.

Dima
+3  A: 

The malloc call you are using is wrong. You should be using:

malloc(sizeof(int) * array_size)

I think.

nc3b
+1  A: 

You're allocating array_size bytes, but you need array_size integers. Try allocating array_size*sizeof(int) bytes in malloc.

John Gordon
+4  A: 

input_array = (int*) malloc(sizeof(int) * (array_size));

It's because the param to malloc is the byte count and int is generally 4 bytes long.

Kotti
+5  A: 

Seeing as its C++ you'd be much better off doing the following:

int main() 
{
  int array_size = 8192;
  int *input_array = new int[array_size];
  random_generator(array_size, input_array);
  delete[] input_array;
}

Edit: or better still:

#include <vector>

int main() 
{
  int array_size = 8192;
  std::vector< int > array;
  array.resize( array_size );
  random_generator(array_size, &array.front());
}

And not even worry about the deallocating :D

Goz
+1  A: 

Everybody else has coverted the mistake in dynamic allocation.
But do you really want dynamic allocation?

int main() {
  int const array_size = 10000;
  int input_array[array_size];
  random_generator(8192, input_array); 
}
Martin York