Iterating vector of pointers results in segmentation fault

Question

I am new to C++ programming. I am trying to implement a Command design pattern using a vector of pointers to some Command instances.

I can iterate over the vector using another aproach (for example using the index to access every element) but I would like to understand why I get the error.

I asume that vector allocates every element contiguously and that every element has the same size.

struct Command {
    virtual void execute(){};
};

struct CommandOne : Command {
    std::string member = "string";
    void execute() override{
        std::cout << member << std::endl;
    }
};

struct CommandTwo : Command {
    void execute() override {
        std::cout << "string" << std::endl;
    }
};

CommandOne commandOne{};
CommandTwo commandTwo{};
std::vector<Command*> commands;

int main (int argc, char** argv){
    commands.push_back(&commandOne);    
    commands.push_back(&commandTwo);    
    Command* ptr_commands = commands.at(0);
    ptr_commands->execute();
    ptr_commands++;
    ptr_commands->execute();
    return 0;
}

The expected output would be:

string
string

But I get:

string
segment fault

Did run your code in a **debugger** to see where that error occurs, then run it again with a breakpoint near that failure so you can step carefully ahead and watch what happens leading up to that point? — tadman, May 24 '19 at 18:36
I don't think you can increment the result of `at`, you need to increment an iterator. Consider using [`for_each`](https://en.cppreference.com/w/cpp/algorithm/for_each) if you can. — tadman, May 24 '19 at 18:38
Good idea! I am still learning that i why I didnt think of that. Thanks for the advice — Rafael Páez Bastida, May 24 '19 at 18:39
You iterated the raw pointer, which is NOT an iterator or part of an array. — Kenny Ostrom, May 24 '19 at 18:40
@RafaelPáezBastida "_I am still learning that i why I didnt think of that._" Consider learning from a [good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). Learning by trial, and error, won't teach you about any amount of possible UB (like the one, you hit here). — Algirdas Preidžius, May 24 '19 at 18:41

tadman · Accepted Answer · 2019-05-24T18:53:05.243

4

The two struct objects here are, for all intents and purposes, located at random positions in memory. The pointer you get back from at refers to one of them, and one of them only. They're not necessarily contiguous in memory. Incrementing that pointer is not a valid operation.

The best way to handle this is to iterate over the vector using the venerable for loop:

for (auto c = commands.begin(); i != commands.end(); ++i) {
  (*c)->execute();
}

In newer C++ this might even look like:

for (auto c : commands) {
  c->execute();
}

You'll also want to consider using new C++ iterator features like for_each:

for_each(commands.begin(), commands.end(), [](Command* c) { c->execute(); });

You'll also want to ensure that anything you allocate with new gets properly released with delete at some point before program termination. By allocating you assume responsibility for cleaning up. Don't neglect that.

As alter points out in the comments below, unique_ptr is a great way of off-loading that responsibility.

edited May 24 '19 at 18:53

answered May 24 '19 at 18:43

tadman

208,517
23
234
262

2

Pushing instances isn't a great option when you need polymorphism. Perhaps `std::unique_ptr` is what you need? – alter_igel May 24 '19 at 18:49
@AlgirdasPreidžius In this example they're pretty trivial, but you're right, there may be better ways. Using `unique_ptr` may be one approach. – tadman May 24 '19 at 18:51
@tadman I deleted my comment, because alter put the gist of my comment in a cleaner way. And, even if the current example is trivial, it **does** require polymorphism, so one wouldn't be able to push instances, even having this example, in mind. – Algirdas Preidžius May 24 '19 at 18:53
@AlgirdasPreidžius I get what you're saying now and I've reworked the answer accordingly. Great points. – tadman May 24 '19 at 18:53

Iterating vector of pointers results in segmentation fault

1 Answers1