From 94e9e57ac5d7308016ce68ea8631c8f2f0d39ddc Mon Sep 17 00:00:00 2001 From: Dino Date: Fri, 28 Aug 2026 21:01:45 -0400 Subject: [PATCH] check if pgood is NULL 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 | 6 ++++-- 7 files changed, 55 insertions(+), 17 deletions(-) diff --git a/ai/default/daiunit.c b/ai/default/daiunit.c index 3e78fc8937..3248dde267 100644 --- a/ai/default/daiunit.c +++ b/ai/default/daiunit.c @@ -2382,13 +2382,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 != NULL) { + priority = pgood->replace_priority; + } if ((city_dest == NULL) || !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..c7c61f39e9 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 != NULL) { + 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..9acc9581a9 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 != NULL) { + 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..9c258b4e18 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 != NULL) { + 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 ae95369ef7..8454fb6cf3 100644 --- a/common/actres.c +++ b/common/actres.c @@ -924,9 +924,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 != NULL) { + 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 0a40b640bf..83cac2b596 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 != NULL) { + 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() : NULL); - 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 != NULL) { + 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..66439e6e2e 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 NULL 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 NULL *************************************************************************/ struct goods_type *goods_from_city_to_unit(const struct city *src, const struct unit *punit) @@ -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 NULL 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 NULL. *************************************************************************/ struct goods_type *unit_current_goods(const struct unit *punit, const struct city *homecity) -- 2.31.0