probably easy question:

I am trying to iterate a list of pairs, get string time to time_t and then compare them to be sorted (bubble sort).

However, using next() on iterator give me segmentation fault on last result and kill my program.

How can I prevent this?

   list<pair<string, Vehicle*>>::iterator it;
    for (list<pair<string, Vehicle*>>::iterator it = allVehicle.begin(); it != allVehicle.end();   it ) {
        time_t time1 = get_the_real_time(it->second->get_date(), it->second->get_time_in());
        cout << time1 << endl;

        // auto next_it = next(it, 1);
        time_t time2 = get_the_real_time(next(it, 1)->second->get_date(), next(it, 1)->second->get_time_in());
    }

Result:

1031729120470
1031729123045
1031732680046
Segmentation fault (core dumped)

I supose that the last round I tries to access a element which do not exist, but I don't know how can I prevent that. Thanks

CodePudding user response：

You should not dereference the end iterator. You can stop the loop when next(it,1) is the end iterator:

auto n = next(it, 1);
if (n == allVehicle.end()) break;
time_t time2 = get_the_real_time(n->second->get_date(), n->second->get_time_in());

or rather adjust the loop condition:

for (auto it = allVehicle.begin(); 
     it != std::prev(allVehicle.end());
       it)

However there is no need to call get_the_real_time twice for each element in the list. You need to call it only once per element:

auto it = allVehicle.begin();
auto prev_time = get_the_real_time(it->second->get_date(), it->second->get_time_in());
  it;

for (  ; it != allVehicle.end();   it) {
    auto current_time = get_the_real_time(it->second->get_date(), it->second->get_time_in());
    // do something with current_time and prev_time
    prev_time = current_time;
}

CodePudding user response：

The reason you get a segmentation is that you iterate past the end of the list.

Solution: Iterate before the starting the loop, and access step back instead when accessing the first element.

Live example

(I have added code just to make it compile)

#include <iostream>
#include <list>

struct Vehicle {
    auto get_time_in() {
        return 0;
    }

    auto get_date() {
        return 0;
    }

};

int main() {
    using namespace std;

    auto get_the_real_time = [](auto x, auto y) {
        return time_t{0};
    };

    auto allVehicles = list<pair<string, Vehicle*>>{
        {"hello", new Vehicle{}}, // Don't handle raw data this way
        {"there", new Vehicle{}}, // Its only for the example code
    };

    if (allVehicles.empty()) {
        return 0;
    }

    // Solution, iterate before entering the loop
    for (auto it = next(allVehicles.begin(), 1); it != allVehicles.end();   it) {
        auto prev_it = prev(it);
        time_t time1 = get_the_real_time(prev_it->second->get_date(), prev_it->second->get_time_in());
        cout << time1 << endl;

        time_t time2 = get_the_real_time(it->second->get_date(), it->second->get_time_in());
    }
}