How to remove Segmentation Fault in My C++ code?

Question

I have written a C++ function that takes array and its size as argument and perform certain operation. When I try to run this code, it shows segmentation fault .Can someone tell me why this code shows a segmentation fault and how can I remove it?

void rearrange(long long *arr, int n) 
{ 
    long long temp[n];
    int max = n-1;
    int min = 0;
    for(int i=0;i<n;i++){
        if(i%2 == 0){
            temp[i] = arr[max];
            max--;
        }
        else{
            temp[i] = arr[min];
            min++;
        }
    }
    
    for(int i=0;i<n;i++){
        arr[i] = temp[i];
    }
}

DId you try to debug the code?`long long temp[n];` is not valid c++. — Quimby, Sep 12 '21 at 17:24
Did you try to compile the code, long long temp[n] should give an error. Have a look at std::vector for arrays with run-time determined sizes. Good thing is you can pass them by explicit reference e.g. rearrange(std::vector& arr). If you do know the size at compile time you could use rearrange(std::array& arr). — Pepijn Kramer, Sep 12 '21 at 17:48

score 0 · Answer 1 · answered Sep 12 '21 at 18:37

0

I tried to call your function like this (without changing anything in it):

void rearrange(long long *arr, int n) 
{
    long long temp[n];
    int max = n - 1;
    int min = 0;
    for(int i = 0;i < n;i++) {
        if(i%2 == 0) {
            temp[i] = arr[max];
            max--;
        } else {
            temp[i] = arr[min];
            min++;
        }
    }
    for(int i = 0;i < n;i++) {
        arr[i] = temp[i];
    }
}

int main()
{
    const int n = 10;
    long long arr[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    rearrange(arr, n);
    return 0;
}

and the compiler g++ 9.3.0 showed no errors and the SIGSEG signal wasn't raised.

But!

Your function has weaknesse. Modern C++ compilers allow you to declare an array the way you did:

long long temp[n];

The weakness is in a fact that the value of the variable n is unknown during compilation. It's a bad C++ way. It's better to do like that (if the C-style arrays are your choice):

void rearrange(long long *arr, int n) 
{
    long long *temp = new long long[n];
    ...
    ...
    ...
    delete temp;
}

In addition, you may have called a function and passed it a pointer arr pointing to a random memory location, i.e. did not allocate memory for the array. For example, if you call your function with such an uninitialized pointer, the OS will initiate a signal SIGSEG (Segmentation Fault):

int main()
{
    const int n = 10;
    long long *arr;    // pointing to any (unknown) memory cell
    rearrange(arr, n);
    return 0;
}

But in this case g++ shows warning message: ‘arr’ is used uninitialized in this function (main).

answered Sep 12 '21 at 18:37

V. Fedulov

41
3

A better way is to use `std::vector` and `push_back` the values. – Thomas Matthews Sep 12 '21 at 19:22
@Thomas Matthews, can you explain why it's better way for this issue? Of coase, the standard C++ library is the powerful thing and provides more high-level programming opportunities. And we can use vector in this issue. But i am sure that the author of this issue is a beginner because he has asked for this question. So what are easer to explain to the beginner: usual pointers - which are the part of the language and does not require any library - or one of the many std library containers? For my opinion, I have shown one of the solutions the closest to author style. – V. Fedulov Sep 12 '21 at 20:47
@V.Fedulov [RAII](https://stackoverflow.com/questions/395123/raii-and-smart-pointers-in-c) to name one. In general, I agree with [Kate Gregory](https://www.youtube.com/watch?v=YnWhqhNdYyk). – Bob__ Sep 13 '21 at 08:13
@V.Fedulov: 1) Memory management (for expansion) is already implemented, *and tested*. You don't have to waste your time developing and debugging. 2) The `std::vector` has a `size` method. When you pass a `std::vector` to a function, you can always get the size. Arrays need to be passed the start of the array and the capacity (maybe even the number of elements). 3) Easier syntax to pass with other functions. 4) The `std::vector` deletes all the memory it has allocated. With dynamically allocated array, **you** have to remember when to delete the memory. Is this enough? – Thomas Matthews Sep 13 '21 at 15:12
Yes, but I asked to explain why vector is the best solution for this particular question. The author, apparently, is no longer interested in this problem. I just wanted to show the nature of the error he encountered by not using the std library. – V. Fedulov Sep 14 '21 at 06:03

score 0 · Answer 2 · answered Sep 12 '21 at 18:40

The problem is most likely an out of bounds array index. This can happen if n is larger than the size of the array, or is a negative number.

One solution would be to use std::vector<long long> &arr instead of long long *arr that way the size() function can be used instead of relying on n to be equal to the array size.

score -2 · Answer 3 · answered Sep 12 '21 at 17:37

-2

first, you cannot make a pointer of an array. The array is always passed by reference.follow this code,

void rearrange(long long arr[], int n)
// do some thing

answered Sep 12 '21 at 17:37

zain ul din

1
1
2
23

1

My advice is to stop using "C" style arrays as much as possible and use std::array/std::vector instead. – Pepijn Kramer Sep 12 '21 at 17:53
2

Please note that this function has exactly the same signature of OP's one. – Bob__ Sep 12 '21 at 18:26
You certainly can allow an array to decay to a pointer; it's logically equivalent either way. – Jeremy Friesner Sep 12 '21 at 20:03

How to remove Segmentation Fault in My C++ code?

3 Answers3