getting segmentation fault when copying arrays using std::copy

Question

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

I'd recommend you should use a `std::vector` instead of raw pointers. — πάντα ῥεῖ, May 08 '19 at 16:44
@yes It is unclear what you are trying to achieve in the while loop. — Vlad from Moscow, May 08 '19 at 16:46
imagine you have a phone book full of addresses of houses.. except there aren't any houses built on those lots and you tell someone to go sleep in one of the bedrooms at that address. Not going to get much sleep — xaxxon, May 08 '19 at 16:48
You can use `vector tab = { 1, 2, 3, 4, 5};`, `std::array tab = {1, 2, 3 , 4, 5};`, `int* tab = new int[5]{1, 2, 3, 4, 5};`. I would advice vectors so you can do `std::copy_n(tab.being(), index, tab1.begin());` or `std:copy(std::begin(tab), std::end(tab) + index, std::being(tab1));` or `std::copy_n(tab.being(), index, tab1.begin());` . After all, you are doing `C++`, not `C`. — acarlstein, May 08 '19 at 17:46

bruno · Answer 1 · 2019-05-08T18:15:27.393

(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++){

sorry i edited my post, tab is initialized and i want to copy its content to tab1. — lyes, May 08 '19 at 16:42
@πάνταῥεῖ but the problem is not tab cause i can print its content by calling print function, the problem is tab1, tab1 is declared as a pointer so can i do `std::copy(tab,tab+index,tab1);` — lyes, May 08 '19 at 16:56
@lyes Well, _undefined behavior_ is _**undefined**_, that means anything can happen. — πάντα ῥεῖ, May 08 '19 at 16:58
@lyes you missed to intialize them, do you understand the problem or not ? I think you imagine to have arrays like I say in my answer — bruno, May 08 '19 at 16:58
@lyes I edited my answer with a corrected version of your code (as I can understand what you wanted) — bruno, May 08 '19 at 17:56

R Sahu · Answer 2 · 2019-05-08T16:51:43.033

1

Problems I see:

You haven't allocated memory for tab and tab1 before using them as though they point to valid memory. That causes undefined behavior.
~~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.

edited May 08 '19 at 16:51

answered May 08 '19 at 16:41

R Sahu

204,454
14
159
270

tab[0] is initialized, just forgot this line, it's why i start indexing at 1, the array is declared as a pointer , you mean i can't start populating it directly ? – lyes May 08 '19 at 16:48
@lyes, Yes, you can. I missed that. – R Sahu May 08 '19 at 16:49

getting segmentation fault when copying arrays using std::copy

2 Answers2