Simon
Simon

Reputation: 241

How to solve segmentation fault error C++

I'm writing a program that gets an array of integers and its logical size. When called it creates a new array containing only the positive numbers from the arrays.

Now, in order to do this I need to write a void type function takes the following parameters:

(int* arr, int arrSize, int** outPosArrPtr, int* outPosArrSizePTR)

I'm supposed to use the pointer int** outPosArrPtr to update the base address of the array containing the positive numbers, and the pointer outPosArrSizePtr to update the array's logical size.

now when I run my code on the xcode compiler the logical size gets updated to a very larger number. So, when I tried to run the program using the online gdb compiler I got the error "Segmentation fault."

From reading what Segmentation fault means I learnt that it means that I'm trying to access memory that "does not belong to me" or memory that is not in the call stack or in the heap portion of the program.

I tried to debug my code by seeing if I was referening any null pointers or see if I was referencing any dangling pointers but it seems like the problem is another one.

My code:

#include <iostream>

typedef int* IntArrPtr;
using namespace std;


int main() {
    int arrSize;
    int *ptrSize;
    ptrSize = &arrSize;

    cout << "How many integers will this array hold:\n ";
    cin >> arrSize;

    IntArrPtr a;
    a = new int[arrSize];

    fillArr(a, arrSize);

    getPosNums4(a, arrSize,&a, ptrSize);
    cout << "The new size in main is: " << arrSize << endl;

    cout <<"The new array with positive integers is:\n";
    /*for(int i =0; i<arrSize;i++) // this runs for a large size of arrSize
        cout<< a[i] << " ";
    cout<<endl; */
    return 0;
}

void fillArr(int a[], int size){
    cout << "Please enter " << size << " Integers separated by spaces\n";
    cout << "Press enter when finished >\n";
    int i;

    for (i=0;i<size;i++)
        cin >> a[i];

}

void getPosNums4(int* arr, int arrSize, int** outPosArrPtr,int* outPosArrSizePtr){
    IntArrPtr newArr;
    newArr = new int[arrSize];
    int i;
    int newIndx = 0;
    outPosArrSizePtr = &newIndx;//initiliaze the pointer.
    for(i=0;i<arrSize;i++){
        if(arr[i] > 0){
            newArr[newIndx] =arr[i];
            newIndx++;
        }
    }
    arrSize = newIndx;
    *outPosArrSizePtr = arrSize;
    cout << "The new size is of *outPosArrSizeptr is: " << *outPosArrSizePtr << endl;

    for(int j=0;j<newIndx;j++)
        outPosArrPtr[j] = &newArr[j];
    delete []newArr;
    newArr = NULL;
    for(int i=0;i<newIndx;i++)
        arr[i] = *outPosArrPtr[i];

}

an example When I run this program on Xcode:

How many integers will this array hold:
 6
Please enter 6 Integers separated by spaces
Press enter when finished >
3 -1 -3 0 6 4
The new size is of *outPosArrSizeptr is: 3
The new array with positive integers is:
The new size in main is: 7445512
The program ended with exit code: 0

Upvotes: 2

Views: 2008

Answers (2)

Matthew Fisher
Matthew Fisher

Reputation: 2336

Modern C++ uses vector rather than manually allocating arrays. Manual allocation is prone to a variety of errors that are very difficult to debug.

The logic in your getPosNums4 method appears to be the trouble. If I understand the requirement, it should look for positive integers in the input array and copy them to a newly allocated output array. Over-allocating the output array is non-optimal but not an actual bug.

void getPosNums4(int* arr, int arrSize, int** outPosArrPtr,int* outPosArrSizePtr){
    IntArrPtr newArr;
    newArr = new int[arrSize];
    int i;
    int newIndx = 0;
    for(i=0;i<arrSize;i++){
        if(arr[i] > 0){
            newArr[newIndx] =arr[i];
            newIndx++;
        }
    }
    *outPosArrSizePtr = newIndx;
    cout << "The new size is of *outPosArrSizeptr is: " << *outPosArrSizePtr << endl;
    *outPosArrPtr = newArr;
}

Note the the newly allocated array will need to be delete[] by the calling function or a memory leak will result.

Here is the same program in modern C++. Note that there is no use of new/delete which saves a lot of misery.

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

int main() {
    vector<int> integer_vector;
    vector<int> positive_vector;

    cout << "Type in integers. Type a Q to continue:" << endl;
    int an_int;
    while(cin >> an_int)
      integer_vector.push_back(an_int);

    for_each(integer_vector.begin(),integer_vector.end(),[&](int const& n){ 
       if(n > 0)
          positive_vector.push_back(n);
    });
    cout <<"The new array with positive integers is:\n";
    for(auto const & element:positive_vector)
        cout<< element << " ";
    cout<<endl; 
    return 0;
}

Upvotes: 0

molbdnilo
molbdnilo

Reputation: 66371

There are quite a few problems there, but the most crucial one is that assigning a value to a function's argument has no effect on the variable whose value you passed as the argument.
It doesn't matter that the argument is a pointer – there is nothing special about pointers.

What I think is happening is that your "copy back and forth" loop (I can't understand what it's supposed to do) in the function is writing outside the input array, causing undefined behaviour and, in this case, overwriting variables in main.

You're overcomplicating your function quite a bit. It should

  • Create a new array
  • Copy the positive values to this array
  • Update the output parameters with the address of this array and its (logical) size

(Think of out parameters as return values and handle them last.)

Something like this:

void getPosNums4(int* arr, int arrSize, int** outPosArrPtr,int* outPosArrSizePtr){
    int* newArr = new int[arrSize];
    int newIndx = 0;
    for (int i = 0; i < arrSize; i++){
        if (arr[i] > 0){
            newArr[newIndx] = arr[i];
            newIndx++;
        }
    }
    *outPosArrPtr = newArr;
    *outPosArrSizePtr = newIndx;
}

You should also not pass pointers to your "originals" for this function to modify, you should use new variables.

int main() {
    int arrSize = 0;
    cout << "How many integers will this array hold:\n ";
    cin >> arrSize;
    int* a = new int[arrSize];
    fillArr(a, arrSize);
    int * positives = nullptr;
    int positiveSize = 0;
    getPosNums4(a, arrSize, &positives, &positiveSize);
    cout << "The new size in main is: " << positiveSize << endl;

    delete [] a;
    delete [] positives;
}

Upvotes: 1

Related Questions