T. Bennett
T. Bennett

Reputation: 11

std::vector segfaulting instead of throwing exception

I am attempting to create a container class for a std::vector, to teach myself a bit more about templates, overloading operators, and managing exceptions.

For the moment, I'm just defining the basic operations. I have a template class listed below; I've overloaded the += and [] operators to push_back the vector with a T and access the elements of the vector directly, respectively. This works as expected.

The += operator does what it's supposed to do, and attempting to use the [] operator on an element out of range will throw the exception as intended.

Here is the prototype class and implementation as it currently stands:

#include <iostream>
#include <vector>
#include <string>

using namespace std;

template <class T>
class Inventory
{
    public:
        void operator += (const T& b)   { backpack.push_back(b); }

        T operator [] (const unsigned& b)
        {
            if (backpack.empty() || backpack.size() < b)
                throw string("Out of Range");
            return backpack[b];
        }

        void operator -= (const unsigned& b)
        {
            if (backpack.empty() || backpack.size() < b)
                throw string("No such element exists.");
            backpack.erase(backpack.begin() + b);
        }

    private:
        vector<int> backpack;
};

int main()
{
    Inventory<int> pack;
    pack += 2;
    pack += 4;
    try
    {
        cout << "It was " << pack[0] << endl;
        cout << "It was " << pack[1] << endl;
        pack -= 0;
        cout << "It is now " << pack[0] << endl;
        //pack -= 1; // Segfaults?
    }
    catch (string e)
    {
        cout << "Error: " << e << endl;
    }
}

The issue is with the -= operator, intended to erase an element at the indicated position on the right hand side. When I stay within the boundaries of the vector, this works as intended; however, I do not get an exception if I specify an out of bounds number to erase; I get a seg-fault instead. I have attempted to determine the exact point the segfault occurs by adding additional print commands:

void operator -= (const unsigned& b)
{
    cout << "In Overload!\n";
    if (backpack.empty() || backpack.size() < b)
    {
        cout << "Exception!\n";
        throw string("No such element exists.");
    }
    backpack.erase(backpack.begin() + b);
}

The "Exception!" line is never reached. The program faults before it can reach that point, even though I should be evaluating for undefined behavior. I believe I'm missing a key component in understanding how this process works. Is there a way I should be writing this so it can throw instead of fault?

Compiling using g++ -std=c++17 -Wall -Wextra -pedantic on Linux x64 architecture.

Upvotes: 1

Views: 1124

Answers (3)

Peter
Peter

Reputation: 36607

You have an "off by one" error in your code.,

Consider what happens if the array is not empty and b == backpack.size() in the code.

 if (backpack.empty() || backpack.size() < b)
            throw string("Out of Range");
 return backpack[b];

In this case, valid indices for elements of backpack are 0 through to backpack.size() - 1.

If b == backpack.size(), the code will NOT throw an exception, and WILL attempt to return backpack[backpack.size()] which gives undefined behaviour.

One possible symptom of undefined behaviour is a "segfault".

One way to avoid the problem is to change the test to backpack.size() <= b.

Upvotes: 1

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

Another alternative is to take advantage of std::vector::at() which will throw a std::out_of_range exception on an out-of-bounds index:

    T operator [] (const unsigned& b)
    {
        try 
        {
           return backpack.at(b);
        }
        catch (std::out_of_range& e)
        {
            throw string("Out of Range");
        }
    }

    void operator -= (const unsigned& b)
    {
        try
        {
            backpack.at(b);
            backpack.erase(backpack.begin() + b);
        }
        catch(std::out_of_range& e)
        {
           throw std::string("No such element exists.");
        }
    }

Live Example

Upvotes: 0

Sam Varshavchik
Sam Varshavchik

Reputation: 118350

Your error checking is off by 1.

if (backpack.empty() || backpack.size() < b)

If the std::vector backpack contains only two values, backpack.size() is going to be 2, and backpack will contain backpack[0] and backpack[1].

Unfortunately, if the index b gets passed in as 2, this code will still attempt to access backpack[2], resulting in undefined behavior.

In fact, the entire if statement can be simply rewritten as:

if (b >= backpack.size())
    throw string("Out of Range");

Upvotes: 1

Related Questions