getting segmentation fault when copying arrays using std::copy

682 Views Asked by At

i'm trying to carry out a pascal triangle, and i need to copy an array to an other, i'm currently using the copy function but getting the famous segmentation fault.

here is my code:

    #include <iostream>
    #include <algorithm> 
    #include <cstring>

    using namespace std;

    void print(const int *tab, const int &nbr){
        for(int i = 0; i<nbr;i++){
          cout << tab[i];
         }

        cout << endl;
     }


     int main()
     {
       int *tab;
       int *tab1;
       int index = 0;
       cout << "enter a number less than or equal to 20!" << endl;
       int number = 40;
       while(number > 20){
         cin >> number;
         cout << endl;
       }

       int a = 1;

       while(index < number){
         tab[0] = 1;
         for(int i=1;i<=index;i++){
            tab[i] = i;
         }
         print(tab,index);
         std::copy(tab,tab+index,tab1);
         index++;
       }

       return 0;
     }

I'm getting the same error with memcpy function , can anyone

2

There are 2 best solutions below

5
On

(before your edition)

getting segmentation fault when copying arrays using std::copy

The problem appears before the std::copy

Having

 int *tab;
 int *tab1;
 ...
 tab[i] = tab1[i];

tab and tab1 are not initialized, they do not point to a block of memory used as an array, so the behavior is undefined (a segmentation fault in your case) each time they are dereferenced

Concerning the code about number you probably wanted something like

int tab[20];
int tab1[20]

Warning in

 for(int i=1;i<=index;i++){

it seems you suppose the first index of an array is 1, while it is 0


A proposal from your code (after your edition) removing undefined behaviors, more some other changes, I commented the modifications.

#include <iostream>
#include <algorithm> 
#include <cstring>

using namespace std;

void print(const int *tab, const int &nbr){
  for(int i = 0; i<nbr;i++){
    cout << tab[i] << ' '; // add a space to separate numbers 
  }

  cout << endl;
}

int main()
{
  int number;

  do { // your case it typically a "do while"
    // print moved inside to clearly indicate the expected input
    // even after a number invalid
    // and also request a  number > 0 else no sence after
    cout << "enter a number between 1 and 20!" << endl;
    if (!(cin >> number)) { // detect the error else if a non number you loop forever
      cerr << "invalid input" << endl;
      cin.clear(); // clear the error

      // bypass invalid input
      string s;

      if (! (cin >> s)) {
        // EOF !
        return -1;
      }
      number = 0; // to reloop
    }
  } while ((number > 20) || (number <= 0));

  int * tab = new int[number]; // added missing initialization
  int * tab1 = new int[number]; // added missing initialization

  for (int index = 0; index < number; ++index) {
    tab[0] = 1;
    for(int i=1; i<=index; i++) {
      tab[i] = i;
    }
    print(tab,index);
    std::copy(tab, tab+index, tab1);
  }

  // free resources
  delete [] tab;
  delete [] tab1;

  return 0;
}

Compilation and execution :

pi@raspberrypi:/tmp $ g++ -pedantic -Wextra -Wall cp.cc
pi@raspberrypi:/tmp $ ./a.out
enter a number between 1 and 20!
aze
invalid input
enter a number between 1 and 20!
-1
enter a number between 1 and 20!
21
enter a number between 1 and 20!
20

1 
1 1 
1 1 2 
1 1 2 3 
1 1 2 3 4 
1 1 2 3 4 5 
1 1 2 3 4 5 6 
1 1 2 3 4 5 6 7 
1 1 2 3 4 5 6 7 8 
1 1 2 3 4 5 6 7 8 9 
1 1 2 3 4 5 6 7 8 9 10 
1 1 2 3 4 5 6 7 8 9 10 11 
1 1 2 3 4 5 6 7 8 9 10 11 12 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 14 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 
1 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 
pi@raspberrypi:/tmp $ 

However in

for (int index = 0; index < number; ++index) {
  tab[0] = 1;
  for(int i=1; i<=index; i++) {
    tab[i] = i;
  }
  print(tab,index);
  std::copy(tab, tab+index, tab1);
}

tab is initialized a lot of times for nothing, it is enough to initialize each entry only one time

std::copy(tab, tab+index, tab1); is useless because tab1 is never used.

It is possible to remove all concerning tab1 and to just have :

tab[0] = 1;
for (int index = 1; index < number; ++index) {
  tab[index] = index;
  print(tab,index);
}

An execution under valgrind to check memory accesses and leaks (tab1 removed) :

pi@raspberrypi:/tmp $ valgrind ./a.out
==16633== Memcheck, a memory error detector
==16633== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==16633== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==16633== Command: ./a.out
==16633== 
enter a number between 1 and 20!
10
1 
1 1 
1 1 2 
1 1 2 3 
1 1 2 3 4 
1 1 2 3 4 5 
1 1 2 3 4 5 6 
1 1 2 3 4 5 6 7 
1 1 2 3 4 5 6 7 8 
==16633== 
==16633== HEAP SUMMARY:
==16633==     in use at exit: 0 bytes in 0 blocks
==16633==   total heap usage: 4 allocs, 4 frees, 22,312 bytes allocated
==16633== 
==16633== All heap blocks were freed -- no leaks are possible
==16633== 
==16633== For counts of detected and suppressed errors, rerun with: -v
==16633== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 3)
pi@raspberrypi:/tmp $ 

Also note you miss to print the last element in print

for(int i = 0; i<nbr;i++){

can be

for(int i = 0; i<=nbr;i++){
2
On

Problems I see:

  1. You haven't allocated memory for tab and tab1 before using them as though they point to valid memory. That causes undefined behavior.

  2. You don't have any code for populating tab with data. Copying from tab to tab1 does not make sense without that. The updated code initializes tab.