CODEWITHSUNDEEP
c++vectoriteratorsegmentation-fault
Bayou
Bayou

Reputation: 3451

C++ std::vector entries are NULL in function, but size remains above zero

It has been a while that I've written code in C/C++, and I've already found an alternative solution to my problem, but I would like to know why the original code doesn't work.

I have a test class which basically only stores a string.

class test {
private:
        std::string name;
public:
        test(std::string name) : name(name) {};
        std::string get_name() { return name; }
};

In main I have a vector which I at one point fill with test objects. The code below simulates irregular usage of the vector vect.

int main(void) {
        std::vector<test *> vect;
        std::vector<test *>::iterator i;

        //* Comment this for a working example
        std::cout << "Searching empty vector" << std::endl;
        i = *(_is_in_vector(vect, std::string("test 3")));
        if (i == vect.end()) {
                std::cout << "Nothing found" << std::endl;
        } // */

        vect.push_back(new test("test 1"));
        vect.push_back(new test("test 2"));
        vect.push_back(new test("test 3"));

        std::cout << "All:" << std::endl;
        i = *(_is_in_vector(vect, std::string("test 3")));
        if (i != vect.end()) {
                std::cout << "Erase " << (*i)->get_name() << std::endl;
                vect.erase(i);
                delete *i;
        }

        i = *(_is_in_vector(vect, std::string("test 3")));
        if (i == vect.end()) {
                std::cout << "Nothing found" << std::endl;
        }

        std::cout << "Left:" << std::endl;
        for (i = vect.begin(); i!=vect.end(); ++i) {
                std::cout << (*i)->get_name() << std::endl;
                delete *i;
        }

        vect.clear();
        return 0;
}

Because searching in the vector for a test object happens multiple times, I've created the function _is_in_vector that searches a test object and returns the iterator to it.

static std::vector<test *>::iterator * _is_in_vector(std::vector<test *> &vect, std::string find) {
        std::string identity = find;
        static std::vector<test *>::iterator i = vect.begin();
        std::cout << "Vect size: " << vect.size() << std::endl;
        for (i; i != vect.end(); ++i) {
                std::string tmp = (*i)->get_name(); /* Segmentation fault after filling vector*/
                if (0 == identity.compare(tmp)) break;
        }
        return &i;
}

The problem is that the code above works when I comment out the Searching empty vector part in main. Once the vector is filled with test objects, I call _is_in_vector a second time. The vector in this function does have three entries, but (*i) all point to NULL.

Output:

Searching empty vector
Vect size: 0
Nothing found
All:
Vect size: 3
Segmentation fault

Expected output:

Searching empty vector
Vect size: 0
Nothing found
All:
Vect size: 3
Erase test 3
Vect size: 2
Nothing found
Left:
test 1
test 2

Upvotes: 0

Views: 610

Answers (1)

Slava
Slava

Reputation: 44268

First of all it is unclear why you need to store test objects by pointer but not by value. If you do need it use smart pointer.

As for your problem, why do you return pointer to an iterator? This is the root cause of your problem - to make &i legal to return you made it static, but static local variables initialized only once and do not change value btw calls - so after first call it pointed to an element in the vector, but after that you added elements and invalidated all iterators including static i hense the segmentation fault. So fix is simple - return iterator by value and make i non static but regular, it is light and it is totally fine to do so.

PS Identifiers starting with _ are illegal in global context, details can be found here What are the rules about using an underscore in a C++ identifier?

So your function actually should look like this:

static std::vector<test *>::iterator  is_in_vector( std::vector<test *> &vect, const std::string &find) 
{
      return std::find_if( vect.begin(), vect.end(), [find]( test *p ) {
          return p->get_name() == find;
      } );
}

assuming the vector should never hold nullptr, if it is the case or to play safe change condition to:

          return p && p->get_name() == find;

Upvotes: 5

Related Questions