Getting segmentation fault in recursive insertion sort

Question

I was trying to write recursive code of insertion sort, but I am getting segmentation fault. Please help me with this.

#include <bits/stdc++.h>

using namespace std;

void insert(vector<int> &v,int temp){
    if(v.size()==0||v[v.size()-1]<=temp){
        v.push_back(temp);
        return;
    }
    int val = v[v.size()-1];
    v.pop_back();
    insert(v, temp);
    v.push_back(val);
    return;
}

void sort(vector<int> &v){
    if(v.size()==1) return;
    int temp = v[v.size()-1];
    v.pop_back();
    sort(v);
    insert(v,temp);
}

int main() {
    int n;
    vector<int> v;
    cin>>n;
    for(int i=0;i<n;i++)
    cin>>v[i];
    sort(v);
    for(int i=0;i<n;i++)
    cout<<v[i];
    return 0;
}

Answer 1

The problem is that the vector v is empty(meaning it has size 0) and you're trying to access its elements which leads to undefined behavior.

vector<int> v; //this creates an empty vector named v
cin>>n;
for(int i=0;i<n;i++)
cin>>v[i]; //this leads to undefined behavior

In the above snippet, the vector v is an empty vector meaning it has no elements. So the expression cin >> v[i], leads to undefined behavior because you're accessing elements at ith index of the vector but there are no elements inside the vector to access.

Undefined behavior means anything¹ can happen including but not limited to the program giving your expected output. But never rely(or make conclusions based) on the output of a program that has undefined behavior.

So the output that you're seeing(maybe seeing) is a result of undefined behavior. And as i said don't rely on the output of a program that has UB. The program may just crash(which is what happens in your case).

So the first step to make the program correct would be to remove UB. Then and only then you can start reasoning about the output of the program.

Solution

To solve this you can specify the size of the vector when creating it as shown below:

int n;
cin>>n;
vector<int> v(n);//create vector of size n

---

^{1For a more technically accurate definition of undefined behavior see this where it is mentioned that: there are no restrictions on the behavior of the program.}

Answer 2

This creates an empty vector<int>:

vector<int> v;

Accessing any elements in it will make your program have undefined behavior. You can instead create the vector<int> with the number of elements you want by supplying n to the constructor, or by calling resize(n) after creating an empty vector<int>. Another option is to create it empty, and then to push_back() or emplace_back elements into it in which case it'll grow dynamically.

Example creating it with the correct number of elements:

int main() {
    int n;

    // verify that extraction of `n` succeeded and that `n > 0`:
    if(!(std::cin >> n && n > 0)) {
        std::cerr << "error\n";
        return 1;
    }

    // create the vector with the correct number of elements
    std::vector<int> v(static_cast<size_t>(n));

    // use a range-based for loop to get a reference to each element:
    for(int& val : v)
        std::cin >> val;    // extract into the referenced `int`

    sort(v);

    // here you can use a range-based for loop to get a copy of each element
    // (which is cheap in this case, since they are `int`s):
    for(int val : v)
       std::cout << val << ' ';
    std::cout << '\n';
}

Demo

Notes:

• #include <bits/stdc++.h> - Never include this non-standard header. Include the standard headers, like iostream and vector

• using namespace std; - Don't do this. Especially not when you provide overloads for functions like sort that will be available if you include bits/stdc++.h and do using namespace std;.

Answer 3

You declared an empty vector

int n;
vector<int> v;

So using the subscript operator in this for loop

for(int i=0;i<n;i++)
cin>>v[i];

invokes undefined behavior.

Instead you could either initially declare the vector with n elements after the value of n was entered as for example

int n;
cin>>n;
vector<int> v( n );
for(int i=0;i<n;i++)
cin>>v[i];

Or you could resize the vector after the value of n becomes known like

int n;
vector<int> v;
cin>>n;
v.resize( n );
for(int i=0;i<n;i++)
cin>>v[i];

At it would be better to declare the variable n as having an unsigned integer type as for example

size_t n;

Instead of the for loop you can use the range based for loop

size_t n = 0;
cin>>n;

std::vector<int> v( n );

for ( auto &item : v ) std;:cin >> item;

Within the function sort you need to change the condition in the if statement at least the following way

void sort( std::vector<int> &v ){
    if ( v.size() < 2 ) return;
    //...

because in general the user can pass an empty vector.

Instead of using expressions like this v[v.size()-1] you could use the member function back. For example

int temp = v.back();

that makes the code more readable.

I would define the functions the following way without redundant return statements

void insert( std::vector<int> &v, int value )
{
    if (v.empty() || not( value < v.back() ) )
    {
        v.push_back( value );
    }
    else
    {
        int tmp = v.back();
        v.pop_back();
        insert( v, value );
        v.push_back( tmp );
    }
}

void sort( std::vector<int> &v )
{
    if (not ( v.size() < 2 ))
    {
        int last = v.back();
        v.pop_back();
        sort( v );
        insert( v, last );
    }
}

Here is a demonstration program.

#include <iostream>
#include <vector>

void insert( std::vector<int> &v, int value )
{
    if (v.empty() || not( value < v.back() ) )
    {
        v.push_back( value );
    }
    else
    {
        int tmp = v.back();
        v.pop_back();
        insert( v, value );
        v.push_back( tmp );
    }
}

void sort( std::vector<int> &v )
{
    if (not ( v.size() < 2 ))
    {
        int last = v.back();
        v.pop_back();
        sort( v );
        insert( v, last );
    }
}

int main()
{
    std::vector<int> v = { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };
    sort( v );

    for (const auto &item : v)
    {
        std::cout << item << ' ';
    }
    std::cout << '\n';
}

The program output is

0 1 2 3 4 5 6 7 8 9

And advice: try to avoid the using directive

using namespace std;

it can be a reason of name collisions and sometimes makes the code unclear that is whether there is used a standard C++ function or a user defined function.