CODEWITHSUNDEEP
c++functionrecursionvectorsegmentation-fault
user8453362
user8453362

Reputation:

Segmentation fault in recursive function, storing result in a vector

I keep getting this seg fault but I have no idea where it came from. Sorry I'm still new to coding.

#include <iostream>
#include <vector>

using namespace std;

vector<int> map(vector<int> v, vector<int>::iterator i, vector<int> result)  { //set i = v.begin() in main
    if (i==v.end()) {
        return result;
    }   else    {
        result.push_back((*i)*(*i));
        i++;
        map(v,i,result);
    }
}

int main()  {
    vector<int> v;
    vector<int> result;

    for (int i=0;i<20;i++)  {
        v.push_back(i);
    }

    vector<int>::iterator it=v.begin();

    result=map(v,it,result);
}

And apparently, I need to add more word because my question is mostly code.

Upvotes: 1

Views: 471

Answers (4)

gsamaras
gsamaras

Reputation: 73366

You pass the vector by value, thus the changes don't persist among function calls.

Pass the vector by reference to achieve this.

Furthermore, you need to return the vector in the else case too.

Moreover, pass v by reference too, in order for the iterator to be OK when yuo check for v.end(). Otherwise it will look in a different copy of v at every function call.

Putting everything together, you get:

vector<int> map(vector<int>& v, vector<int>::iterator i, vector<int>& result)  { 
    if (i==v.end()) {
        return result;
    }   else    {
        result.push_back((*i)*(*i));
        i++;
        return map(v,i,result);
    }
}

Upvotes: 2

Peter
Peter

Reputation: 36597

One potential cause of a crash will be because the recursive call is not followed by (or part of) a return statement. When the caller accesses the return value, the result is undefined behaviour.

Even ignoring that, the arguments are passed by value. Any changes made to the vector will therefore not be visible to the caller. More significantly, the test i == v.end() will also have undefined behaviour (another potential cause of a crash) because i and v.end() are not iterators obtained from the same container (i is an iterator from the vector in main(), and v is a copy of that vector - which has a completely different set of iterators).

Lastly, std::map is a templated type in the standard library. Having a function named map() - particularly when a using namespace std is in play, is likely to confuse programmers, if not result in ambiguity to the compiler.

Upvotes: 0

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

There are two problems:

Since you are passing the first parameter (std::vector) by value, each call to map is using a different vector than the original. Thus the iterator you're passing is not compatible with the passed-in vector and your program will exhibit undefined behavior.

To fix this problem, pass the std::vector by reference, not by value. Since you are also not changing the vector within the function, pass by const reference:

vector<int> map(const vector<int>& v, vector<int>::iterator i, vector<int> result)

Now the iterator is iterating over the actual vector that was passed in, not a temporary copy of the vector.

The second issue is that you are not returning a value from the map function. Not returning a value from a function that is supposed to return a value is undefined behavior.

To fix the issue, remove the else statement (to avoid any compiler warnings) and return the value from the function:

vector<int> map(const vector<int>& v, vector<int>::iterator i, vector<int> result) 
{   
   if (i == v.end()) 
      return result;
   result.push_back((*i)*(*i));
   i++;
   return map(v, i, result);
}

Upvotes: 1

xaxxon
xaxxon

Reputation: 19761

The problem is almost certainly because you're not returning a value from your recursive function in many situations:

vector<int> map(vector<int> v, vector<int>::iterator i, vector<int> result)  { //set i = v.begin() in main
    if (i==v.end()) {
        return result;
    }   else    {
        result.push_back((*i)*(*i));
        i++;
        map(v,i,result);
       /** NO RETURN VALUE HERE **/
    }
}

Instead, make the last line be: 

return map(v,i,result);

Ideally you won't pass in vectors by value, but that won't cause your program to crash, just run more slowly.

Upvotes: 0

Related Questions