Skip to content

Commit

Permalink
Fix a class of crashes in the city list
Browse files Browse the repository at this point in the history
Modifying a city can (somehow) change the selection, upon which selected_cities
is modified. Modifying a QList invalidates iterators, thus we were triggering
undefined behavior when doing this in loops. Copy the list before doing
anything with it, which avoids any invalidation (since we work on the copy).
  • Loading branch information
lmoureaux authored and jwrober committed Dec 26, 2022
1 parent 7ed28fc commit b45fdf4
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions client/cityrep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,9 @@ void city_widget::clear_worlist()
struct worklist empty;
worklist_init(&empty);

for (auto *pcity : qAsConst(selected_cities)) {
const auto saved_selection =
selected_cities; // Copy to avoid invalidations
for (auto *pcity : saved_selection) {
Q_ASSERT(pcity != nullptr);
city_set_worklist(pcity, &empty);
}
Expand All @@ -435,7 +437,9 @@ void city_widget::clear_worlist()
*/
void city_widget::buy()
{
for (auto *pcity : qAsConst(selected_cities)) {
const auto saved_selection =
selected_cities; // Copy to avoid invalidations
for (auto *pcity : saved_selection) {
Q_ASSERT(pcity != nullptr);
cityrep_buy(pcity);
}
Expand Down Expand Up @@ -642,7 +646,9 @@ void city_widget::display_list_menu(const QPoint)
}
city_list_iterate_end;

for (auto *pcity : qAsConst(selected_cities)) {
const auto saved_selection =
selected_cities; // Copy to avoid invalidations
for (auto *pcity : saved_selection) {
if (nullptr != pcity) {
switch (m_state) {
case CHANGE_PROD_NOW:
Expand Down Expand Up @@ -687,13 +693,10 @@ void city_widget::display_list_menu(const QPoint)
}
break;
case CMA:
if (nullptr != pcity) {
if (CMA_NONE == id) {
cma_release_city(pcity);
} else {
cma_put_city_under_agent(pcity,
cmafec_preset_get_parameter(id));
}
if (CMA_NONE == id) {
cma_release_city(pcity);
} else {
cma_put_city_under_agent(pcity, cmafec_preset_get_parameter(id));
}

break;
Expand Down

0 comments on commit b45fdf4

Please sign in to comment.