tags:

views:

253

answers:

2

I am writing a test program to understand vector's better. In one of the scenarios, I am trying to insert a value into the vector at a specified position. The code compiles clean. However, on execution, it is throwing a Segmentation Fault from the v8.insert(..) line (see code below). I am confused. Can somebody point me to what is wrong in my code?

#define UNIT_TEST(x) assert(x)
#define ENSURE(x) assert(x)

#include <vector>
typedef std::vector< int >                     intVector;
typedef std::vector< int >::iterator           intVectorIterator;
typedef std::vector< int >::const_iterator     intVectorConstIterator;


intVectorIterator find( intVector v, int key );
void test_insert();

intVectorIterator
find( intVector v, int key )
{
    for( intVectorIterator it = v.begin(); it != v.end(); ++it )
    {
        if( *it == key )
        {
            return it;
        }
    }

    return v.end();
}

void
test_insert()
{
    const int values[] = {10, 20, 30, 40, 50};
    const size_t valuesLength = sizeof( values ) / sizeof( values[ 0 ] );
    size_t index = 0;
    const int insertValue = 5;

    intVector v8;
    for( index = 0; index < valuesLength; ++index )
    {
        v8.push_back( values[ index ] );
    }

    ENSURE( v8.size() == valuesLength );
    for( index = 0; index < valuesLength; ++index )
    {
        printf( "index = %u\n", index );

        intVectorIterator it = find( v8, values[ index ] );
        ENSURE( it != v8.end() );
        ENSURE( *it == values[ index ] );

        // intVectorIterator itToInsertedItem = 
        v8.insert( it, insertValue );                      // line 51
        // UNIT_TEST( *itToInsertedItem == insertValue );
    }
}

int main()
{
    test_insert();
    return 0;
}

$ ./a.out
index = 0
Segmentation Fault (core dumped)

(gdb) bt
#0  0xff3a03ec in memmove () from /platform/SUNW,T5140/lib/libc_psr.so.1
#1  0x00012064 in std::__copy_move_backward<false, true, std::random_access_iterator_tag>::__copy_move_b<int> (__first=0x23e48, __last=0x23450, __result=0x23454)
    at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/stl_algobase.h:575
#2  0x00011f08 in std::__copy_move_backward_a<false, int*, int*> (__first=0x23e48, __last=0x23450, __result=0x23454)
    at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/stl_algobase.h:595
#3  0x00011d00 in std::__copy_move_backward_a2<false, int*, int*> (__first=0x23e48, __last=0x23450, __result=0x23454)
    at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/stl_algobase.h:605
#4  0x000119b8 in std::copy_backward<int*, int*> (__first=0x23e48, __last=0x23450, __result=0x23454) at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/stl_algobase.h:640
#5  0x000113ac in std::vector<int, std::allocator<int> >::_M_insert_aux (this=0xffbfeba0, __position=..., __x=@0xffbfebac)
    at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/vector.tcc:308
#6  0x00011120 in std::vector<int, std::allocator<int> >::insert (this=0xffbfeba0, __position=..., __x=@0xffbfebac)
    at /local/gcc/4.4.1/lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/bits/vector.tcc:126
#7  0x00010bc0 in test_insert () at vector_insert_test.cpp:51
#8  0x00010c48 in main () at vector_insert_test.cpp:58
(gdb) q
+7  A: 

You're passing v8 by value to your find function. The iterator returned thus points into the copy of the vector, which is out of scope after the find call returns. Try passing by (const) reference, or better yet, just use std::find.

Kristo
Thanks for catching a very crucial mistake.
ArunSaha
+2  A: 

Your program is broken because it is missing two punctuation marks. :)

As written, when you call your own version of find(), a copy is made of your intVector, and that's used by the function. When find() returns an iterator, it's an iterator of the copy, not of v8. So when you call v8.insert() with another vector's iterator, of course it's going to break.

The solution you're looking for passes the intVector by reference to find(). (i.e. Add two '&' characters)

The better solution is just to reuse std::find().

Karmastan
Thanks for pointing out the mistake.
ArunSaha