From 8fe49f712fb080aef87d4da9e47576510d66e89b Mon Sep 17 00:00:00 2001 From: Dino Date: Fri, 28 Aug 2026 21:35:48 -0400 Subject: [PATCH] check if pgood is nullptr before dereferencing it RM #2179 --- ai/default/daiunit.c | 7 ++++++- client/gui-sdl2/menu.c | 7 ++++++- client/gui-sdl3/menu.c | 7 ++++++- client/text.c | 22 ++++++++++++++-------- common/actres.c | 7 ++++++- common/aicore/caravan.c | 16 +++++++++++++--- common/traderoutes.c | 8 +++++--- 7 files changed, 56 insertions(+), 18 deletions(-) diff --git a/ai/default/daiunit.c b/ai/default/daiunit.c index 074b9ca43e..bb60e16950 100644 --- a/ai/default/daiunit.c +++ b/ai/default/daiunit.c @@ -2349,13 +2349,18 @@ static void dai_manage_caravan(struct ai_type *ait, struct player *pplayer, /* We check to see if our current goal is feasible */ struct city *city_dest = tile_city(punit->goto_tile); struct goods_type *pgood = unit_current_goods(punit, homecity); + int priority = 1; + + if (pgood != nullptr) { + priority = pgood->replace_priority; + } if ((city_dest == nullptr) || !pplayers_allied(unit_owner(punit), city_dest->owner) || (unit_data->task == AIUNIT_TRADE && !(can_cities_trade(homecity, city_dest) && can_establish_trade_route(homecity, city_dest, - pgood->replace_priority))) + priority))) || (unit_data->task == AIUNIT_WONDER /* Helping the (new) production is illegal. */ && !city_production_gets_caravan_shields(&city_dest->production)) diff --git a/client/gui-sdl2/menu.c b/client/gui-sdl2/menu.c index f1957b3376..18cc420776 100644 --- a/client/gui-sdl2/menu.c +++ b/client/gui-sdl2/menu.c @@ -1139,8 +1139,13 @@ void real_menus_update(void) punit->carrying, TRUE); struct goods_type *pgood = unit_current_goods(punit, homecity); + int priority = 1; - if (can_establish_trade_route(homecity, pcity, pgood->replace_priority)) { + if (pgood != nullptr) { + priority = pgood->replace_priority; + } + + if (can_establish_trade_route(homecity, pcity, priority)) { fc_snprintf(cbuf, sizeof(cbuf), _("%s With %s ( %d one time bonus + %d trade ) (R)"), action_id_name_translation(ACTION_TRADE_ROUTE), diff --git a/client/gui-sdl3/menu.c b/client/gui-sdl3/menu.c index 70e2149ca9..6b89871114 100644 --- a/client/gui-sdl3/menu.c +++ b/client/gui-sdl3/menu.c @@ -1135,8 +1135,13 @@ void real_menus_update(void) punit->carrying, TRUE); struct goods_type *pgood = unit_current_goods(punit, homecity); + int priority = 1; - if (can_establish_trade_route(homecity, pcity, pgood->replace_priority)) { + if (pgood != nullptr) { + priority = pgood->replace_priority; + } + + if (can_establish_trade_route(homecity, pcity, priority)) { fc_snprintf(cbuf, sizeof(cbuf), _("%s With %s ( %d one time bonus + %d trade ) (R)"), action_id_name_translation(ACTION_TRADE_ROUTE), diff --git a/client/text.c b/client/text.c index f7e12acd38..77a698a466 100644 --- a/client/text.c +++ b/client/text.c @@ -365,14 +365,20 @@ const char *popup_info_text(struct tile *ptile) struct city *hcity = game_city_by_number(pfocus_unit->homecity); if (utype_can_do_action(unit_type_get(pfocus_unit), ACTION_TRADE_ROUTE) - && can_cities_trade(hcity, pcity) - && can_establish_trade_route(hcity, pcity, - unit_current_goods(pfocus_unit, - hcity)->replace_priority)) { - /* TRANS: "Trade from Warsaw: 5" */ - astr_add_line(&str, _("Trade from %s: %d"), - city_name_get(hcity), - trade_base_between_cities(hcity, pcity)); + && can_cities_trade(hcity, pcity)) { + struct goods_type *pgood = + unit_current_goods(pfocus_unit, hcity); + int priority = 1; + + if (pgood != nullptr) { + priority = pgood->replace_priority; + } + if (can_establish_trade_route(hcity, pcity, priority)) { + /* TRANS: "Trade from Warsaw: 5" */ + astr_add_line(&str, _("Trade from %s: %d"), + city_name_get(hcity), + trade_base_between_cities(hcity, pcity)); + } } } unit_list_iterate_end; } diff --git a/common/actres.c b/common/actres.c index 0ee96f3e7b..35ccf0775d 100644 --- a/common/actres.c +++ b/common/actres.c @@ -925,9 +925,14 @@ enum fc_tristate actres_possible(const struct civ_map *nmap, * entering the market place. */ if (result == ACTRES_TRADE_ROUTE) { struct goods_type *pgood = unit_current_goods(actor->unit, homecity); + int priority = 1; + + if (pgood != nullptr) { + priority = pgood->replace_priority; + } if (!can_establish_trade_route(homecity, target->city, - pgood->replace_priority)) { + priority)) { return TRI_NO; } } diff --git a/common/aicore/caravan.c b/common/aicore/caravan.c index 5b1640970a..53f9c3df35 100644 --- a/common/aicore/caravan.c +++ b/common/aicore/caravan.c @@ -295,6 +295,11 @@ static int one_city_trade_benefit(const struct city *pcity, bool countloser, int newtrade) { int losttrade = 0; + int priority = 1; + + if (pgood != nullptr) { + priority = pgood->replace_priority; + } /* If the city is owned by someone else, we don't benefit from the new trade (but we might still lose from a broken trade route) */ @@ -308,8 +313,7 @@ static int one_city_trade_benefit(const struct city *pcity, } else { struct trade_route_list *would_remove = (countloser ? trade_route_list_new() : nullptr); - int oldtrade = city_trade_removable(pcity, pgood->replace_priority, - would_remove); + int oldtrade = city_trade_removable(pcity, priority, would_remove); /* If we own the city, the trade benefit is only by how much better we are than the old trade route */ @@ -350,6 +354,12 @@ static double trade_benefit(const struct player *caravan_owner, const struct goods_type *pgood, const struct caravan_parameter *param) { + int priority = 1; + + if (pgood != nullptr) { + priority = pgood->replace_priority; + } + /* Do we care about trade at all? */ if (!param->consider_trade) { return 0; @@ -357,7 +367,7 @@ static double trade_benefit(const struct player *caravan_owner, /* First, see if a new route is made. */ if (!can_cities_trade(src, dest) - || !can_establish_trade_route(src, dest, pgood->replace_priority)) { + || !can_establish_trade_route(src, dest, priority)) { return 0; } if (max_trade_routes(src) <= 0 || max_trade_routes(dest) <= 0) { diff --git a/common/traderoutes.c b/common/traderoutes.c index c81a0d743b..1d241e663b 100644 --- a/common/traderoutes.c +++ b/common/traderoutes.c @@ -736,10 +736,11 @@ bool city_receives_goods(const struct city *pcity, /*********************************************************************//** Fond out goods type for the new trade route + May return nullptr if city can't provide any goods. @param src City to start traderoute from @param punit Unit to carry the goods - @return Goods to carry + @return Goods to carry, may be nullptr *************************************************************************/ struct goods_type *goods_from_city_to_unit(const struct city *src, const struct unit *punit) @@ -763,7 +764,7 @@ struct goods_type *goods_from_city_to_unit(const struct city *src, } goods_type_iterate_end; if (i == 0) { - return NULL; + return nullptr; } return potential[fc_rand(i)]; @@ -771,10 +772,11 @@ struct goods_type *goods_from_city_to_unit(const struct city *src, /*********************************************************************//** What goods unit would provide if it established trade route now? + May return nullptr if city can't provide any goods. @param punit Unit to establish the trade route @param homecity Homecity of the unit. Can be used speculatively. - @return What good unit would provide + @return What good unit would provide, may be nullptr *************************************************************************/ struct goods_type *unit_current_goods(const struct unit *punit, const struct city *homecity) -- 2.31.0