Segmentation fault on creating matrices

Question

I was practicing on c++ on some tutorials and I encountered on a tutorial that creates matrices, I wanted something more from it and I modified it, I dont know matrices at all cuz I didnt learn them yet at school but this code below sometimes works sometimes not.
When it doesn't work I usually get: Segmentation fault.
why does this happen ?

before it happened everytime but after i gave a 0 value to variable line and member on the beginning it doesnt happen anymore, but still if I type exc
Line: 10
Member: 9

it gives:

1 1 1 1 1 1 1 1 1

1 2 3 4 5 1 7 8 9
Segmentation fault
and stopes.
Can anyone explain me this ?
thank you !

 #include <iostream>
 #include <iomanip>
 using namespace std;

 int main()
 {
     int line=0,member=0;
     int i,j,matrice[line][member];

     cout << "\nLine: ";
         cin >> line;

     cout << "Member: ";
     cin >> member;

     cout << "\nCreated Matrice: \n" << endl;

         for (i=0;i<line;i++)
         {
             for (j=0;j<member;j++)
             {
                 matrice[i][j]=i*j+1;
                 cout << setw(5) << matrice[i][j];
             }
             cout << "\n\n";
         }
 return 0;
 }

Answer 1

int line=0,member=0;
int i,j,matrice[line][member];

This line shouldn't compile. In standard C++,

1. arrays of 0 size are not allowed
   
2. array sizes must be constant expressions

It appears that your compiler allows these as extensions. In any case when you later input line and member your array size doesn't change. You should define your array after you've input these numbers. But the array must be dynamically allocated (better yet, use vectors)

#include <vector>
//...
int line, member;
cin >> line  >> member;
vector<vector<int> > matrix(line, vector<int>(member));

or if you don't want to use vector for educational purposes, do this:

int line, member;
int ** matrix;
cin >> line  >> member;
matrix = new int*[line];
for(int i = 0; i < line; ++i)
   matrix[i] = new int[member];

Don't forget to free the matrix.

for(int i = 0; i < line; ++i)
   delete [] matrix[i];
delete [] matrix;

I suggest that you should read a good C++ book

HTH

Answer 2

The matrice array is initialized with a size of [0][0], which are the values of line and member. Since you override the values with the inputted values, the bounds used in the for loops are invalid.

i.e. You are accessing items out of the array's bounds.

You may want to use new to dynamically create arrays, or just use std::vector which resizes itself.

Also, it is not standard, but if your compiler supports it, you can use variable-length arrays. They behave like regular arrays but are allocated using a runtime-computed value :

int line=0,member=0;
int i,j;

cout << "\nLine: ";
cin >> line;

cout << "Member: ";
cin >> member;

int matrice[line][member];

You should also check for the inputted values, since C++ does not allows zero-size arrays (And it wouldn't make sense in your program anyway.)

Answer 3

You are using dynamic array without allocating memory using malloc or similar. That is in your line int i,j,matrice[line][member]; is not an array with constant size thus memory should be dynamically allocated. Or use a constant matix size as poster above suggested.

Answer 4

I agree with other comments that using vectors is a much safer way to solve your problem: using arrays directly is definitely error-prone. Of course, if your exercise requires using arrays, then you should use arrays.

Regarding the performance, I have written a small test using g++ on Ubuntu 10.04. Running

g++ --version

I get

g++ (Ubuntu 4.4.3-4ubuntu5) 4.4.3

My test program creates a 100x100 matrix and sets each element to some value. It first has a few declarations:

#include <vector>
#include <iostream>
#include "util.h" // Timer utilities.

#define LINE_COUNT (100) // The number of lines.
#define COL_COUNT (100) // The number of columns.

#define REPETITIONS (100000) // Number of repetitions for each test.

using namespace std;

Then I have the test using vectors:

void use_vectors()
{
  int line   = LINE_COUNT;
  int member = COL_COUNT;
  vector<vector<int> > matrix(line, vector<int>(member));

  // Set data.
  for (int i = 0; i < line; i++)
  {
    for (int j = 0; j < member; j++)
    {
      matrix[i][j] = -5;
    }
  }
}

Then I have a function to perform the same test (create matrix and set values) using arrays:

void use_arrays()
{
  int line   = LINE_COUNT;
  int member = COL_COUNT;
  int **matrix; 

  matrix = new int * [line];
  for (int i = 0; i < line; i++)
  {
     matrix[i] = new int[member];
  }

  // Set data.
  for (int i = 0; i < line; i++)
  {
    for (int j = 0; j < member; j++)
    {
      matrix[i][j] = -5;
    }
  }

  for (int i = 0; i < line; ++i)
  {
     delete [] matrix[i];
  }

  delete [] matrix;
}

The main program repeats both tests, and records the time needed for each of them. Here is the main program:

main()
{
  long int es = 0;
  long int eu = 0;
  start_timer();
  for (int i = 0; i < REPETITIONS; i++)
  {
    use_vectors();
  }
  stop_timer();
  es = elapsed_sec();
  eu = elapsed_usec();
  cout << "Vectors needed: " << es << " sec, " << eu << " usec" << endl;

  start_timer();
  for (int i = 0; i < REPETITIONS; i++)
  {
    use_arrays();
  }
  stop_timer();
  es = elapsed_sec();
  eu = elapsed_usec();
  cout << "Arrays needed: " << es << " sec, " << eu << " usec" << endl;
}

The timer functions are based on the library function gettimeofday() (see e.g. http://linux.die.net/man/2/gettimeofday).

The result is the following:

Vectors needed: 24 sec, 624416 usec
Arrays needed:  10 sec, 16970 usec

So it seems that vectors do have some overhead wrt to arrays. Or can I do something to improve the performance of vectors? I checked my benchmark code a few times and it seems to me I got it right.

Anyway, I would by no means advise using arrays just to gain performance unless it really makes a big difference in your application.

Answer 5

You want to allocate memory dynamically. Then, Use Dynamic allocation like this:

 #include <iostream>
 #include <iomanip>
 using namespace std;

 int main()
 {
     int line=0,member=0;
     int i,j;
     int **matrice; //Define matrice as a 2D array(a Matrix)

     cout << "\nLine: ";
         cin >> line;

     cout << "Member: ";
     cin >> member;

     //start of dynamic allocation
     matrice=new int*[line];
     for (i=0;i<line;i++)
           matrice[i]=new int[member];
     //End of dynamic allocation

     cout << "\nCreated Matrice: \n" << endl;

         for (i=0;i<line;i++)
         {
             for (j=0;j<member;j++)
             {
                 matrice[i][j]=i*j+1;
                 cout << setw(5) << matrice[i][j];
             }
             cout << "\n\n";
         }
 delete[] matrice;  //Releasing allocated memory
 return 0;
 }