From 30cd176c8f5a0ed6560c3068e78573697e0f30a3 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sat, 21 Jan 2023 05:23:13 +0200 Subject: [PATCH 2/2] Retire unit_activity_text() As not re-entrant, it caused client crashes. - Convert all callers to use re-entrant unit_activity_astr() - Remove the function See osdn #46559 Signed-off-by: Marko Lindqvist --- client/gui-qt/dialogs.cpp | 13 +++++++++---- client/gui-sdl/citydlg.c | 6 +++++- client/gui-sdl/dialogs.c | 19 ++++++++++++++++--- client/gui-sdl/mapview.c | 6 +++++- client/gui-sdl2/citydlg.c | 6 +++++- client/gui-sdl2/dialogs.c | 21 +++++++++++++++++---- client/gui-sdl2/mapview.c | 5 ++++- client/gui-xaw/dialogs.c | 6 +++++- client/text.c | 7 +++++-- common/unit.c | 15 --------------- common/unit.h | 1 - 11 files changed, 71 insertions(+), 34 deletions(-) diff --git a/client/gui-qt/dialogs.cpp b/client/gui-qt/dialogs.cpp index 1d8f77ddb1..2badf448da 100644 --- a/client/gui-qt/dialogs.cpp +++ b/client/gui-qt/dialogs.cpp @@ -3140,11 +3140,16 @@ void units_select::paint(QPainter *painter, QPaintEvent *event) f_size = &point_size; } if (highligh_num != -1 && highligh_num < unit_list.count()) { + struct astring addition = ASTRING_INIT; + punit = unit_list.at(highligh_num); - /* TRANS: HP - hit points */ - str2 = QString(_("%1 HP:%2/%3")).arg(QString(unit_activity_text(punit)), - QString::number(punit->hp), - QString::number(unit_type_get(punit)->hp)); + unit_activity_astr(punit, &addition); + + // TRANS: HP - hit points + str2 = QString(_("%1 HP:%2/%3")).arg(QString(astr_str(&addition)), + QString::number(punit->hp), + QString::number(unit_type_get(punit)->hp)); + astr_free(&addition); } str = QString(PL_("%1 unit", "%1 units", unit_list_size(utile->units))) diff --git a/client/gui-sdl/citydlg.c b/client/gui-sdl/citydlg.c index 7bcc2c6e80..391c917e56 100644 --- a/client/gui-sdl/citydlg.c +++ b/client/gui-sdl/citydlg.c @@ -27,6 +27,7 @@ #include /* utility */ +#include "astring.h" #include "bitvector.h" #include "fcintl.h" #include "log.h" @@ -710,10 +711,12 @@ static void create_present_supported_units_widget_list(struct unit_list *pList) unit_list_iterate(pList, pUnit) { const char *vetname; + struct astring addition = ASTRING_INIT; pUType = unit_type_get(pUnit); vetname = utype_veteran_name_translation(pUType, pUnit->veteran); pHome_City = game_city_by_number(pUnit->homecity); + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), "%s (%d,%d,%s)%s%s\n%s\n(%d/%d)\n%s", utype_name_translation(pUType), pUType->attack_strength, @@ -721,9 +724,10 @@ static void create_present_supported_units_widget_list(struct unit_list *pList) move_points_text(pUType->move_rate, FALSE), (vetname != NULL ? "\n" : ""), (vetname != NULL ? vetname : ""), - unit_activity_text(pUnit), + astr_str(&addition), pUnit->hp, pUType->hp, pHome_City ? pHome_City->name : Q_("?homecity:None")); + astr_free(&addition); if (pCityDlg->page == SUPPORTED_UNITS_PAGE) { int pCity_near_dist; diff --git a/client/gui-sdl/dialogs.c b/client/gui-sdl/dialogs.c index 876c617fa6..246d4e1750 100644 --- a/client/gui-sdl/dialogs.c +++ b/client/gui-sdl/dialogs.c @@ -27,6 +27,7 @@ #include /* utility */ +#include "astring.h" #include "bitvector.h" #include "fcintl.h" #include "log.h" @@ -1110,6 +1111,9 @@ void unit_select_dialog_popup(struct tile *ptile) vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if (unit_owner(pUnit) == client.conn.playing) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf , sizeof(cBuf), _("Contact %s (%d / %d) %s(%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), pUnit->hp, pUnitType->hp, @@ -1117,7 +1121,8 @@ void unit_select_dialog_popup(struct tile *ptile) pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); + astr_str(&addition)); + astr_free(&addition); } else { int att_chance, def_chance; @@ -1875,6 +1880,9 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if (unit_owner(pUnit) == client.conn.playing) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), _("Activate %s (%d / %d) %s (%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), @@ -1883,7 +1891,8 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); + astr_str(&addition)); + astr_free(&addition); create_active_iconlabel(pBuf, pWindow->dst, pStr, cBuf, adv_unit_select_callback); @@ -1994,6 +2003,9 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if ((pCity && city_owner(pCity) == client.conn.playing) || (unit_owner(pUnit) == client.conn.playing)) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), _("Activate %s (%d / %d) %s (%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), @@ -2002,7 +2014,8 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); + astr_str(&addition)); + astr_free(&addition); create_active_iconlabel(pBuf, pWindow->dst, pStr, cBuf, adv_unit_select_callback); diff --git a/client/gui-sdl/mapview.c b/client/gui-sdl/mapview.c index 25cfb4c05e..20851f9a59 100644 --- a/client/gui-sdl/mapview.c +++ b/client/gui-sdl/mapview.c @@ -778,6 +778,8 @@ void redraw_unit_info_label(struct unit_list *punitlist) pDock = pInfo_Window; n = 0; unit_list_iterate(pTile->units, aunit) { + struct astring addition = ASTRING_INIT; + if (aunit == pUnit) { continue; } @@ -785,6 +787,7 @@ void redraw_unit_info_label(struct unit_list *punitlist) pUType = unit_type_get(aunit); vetname = utype_veteran_name_translation(pUType, aunit->veteran); pHome_City = game_city_by_number(aunit->homecity); + unit_activity_astr(aunit, &addition); fc_snprintf(buffer, sizeof(buffer), "%s (%d,%d,%s)%s%s\n%s\n(%d/%d)\n%s", utype_name_translation(pUType), pUType->attack_strength, @@ -792,9 +795,10 @@ void redraw_unit_info_label(struct unit_list *punitlist) move_points_text(pUType->move_rate, FALSE), (vetname != NULL ? "\n" : ""), (vetname != NULL ? vetname : ""), - unit_activity_text(aunit), + astr_str(&addition), aunit->hp, pUType->hp, pHome_City ? city_name_get(pHome_City) : Q_("?homecity:None")); + astr_free(&addition); pBuf_Surf = create_surf(tileset_full_tile_width(tileset), tileset_full_tile_height(tileset), SDL_SWSURFACE); diff --git a/client/gui-sdl2/citydlg.c b/client/gui-sdl2/citydlg.c index a7d5a99fe8..7efa58e068 100644 --- a/client/gui-sdl2/citydlg.c +++ b/client/gui-sdl2/citydlg.c @@ -31,6 +31,7 @@ #endif /* SDL2_PLAIN_INCLUDE */ /* utility */ +#include "astring.h" #include "bitvector.h" #include "fcintl.h" #include "log.h" @@ -713,10 +714,12 @@ static void create_present_supported_units_widget_list(struct unit_list *pList) unit_list_iterate(pList, pUnit) { const char *vetname; + struct astring addition = ASTRING_INIT; pUType = unit_type_get(pUnit); vetname = utype_veteran_name_translation(pUType, pUnit->veteran); pHome_City = game_city_by_number(pUnit->homecity); + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), "%s (%d,%d,%s)%s%s\n%s\n(%d/%d)\n%s", utype_name_translation(pUType), pUType->attack_strength, @@ -724,9 +727,10 @@ static void create_present_supported_units_widget_list(struct unit_list *pList) move_points_text(pUType->move_rate, FALSE), (vetname != NULL ? "\n" : ""), (vetname != NULL ? vetname : ""), - unit_activity_text(pUnit), + astr_str(&addition), pUnit->hp, pUType->hp, pHome_City ? pHome_City->name : Q_("?homecity:None")); + astr_free(&addition); if (pCityDlg->page == SUPPORTED_UNITS_PAGE) { int pCity_near_dist; diff --git a/client/gui-sdl2/dialogs.c b/client/gui-sdl2/dialogs.c index 97874b23a7..5bc309a241 100644 --- a/client/gui-sdl2/dialogs.c +++ b/client/gui-sdl2/dialogs.c @@ -31,6 +31,7 @@ #endif /* SDL2_PLAIN_INCLUDE */ /* utility */ +#include "astring.h" #include "bitvector.h" #include "fcintl.h" #include "log.h" @@ -1115,6 +1116,9 @@ void unit_select_dialog_popup(struct tile *ptile) vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if (unit_owner(pUnit) == client.conn.playing) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf , sizeof(cBuf), _("Contact %s (%d / %d) %s(%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), pUnit->hp, pUnitType->hp, @@ -1122,7 +1126,8 @@ void unit_select_dialog_popup(struct tile *ptile) pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); + astr_str(&addition)); + astr_free(&addition); } else { int att_chance, def_chance; @@ -1878,6 +1883,9 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if (unit_owner(pUnit) == client.conn.playing) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), _("Activate %s (%d / %d) %s (%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), @@ -1886,8 +1894,9 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); - + astr_str(&addition)); + astr_free(&addition); + create_active_iconlabel(pBuf, pWindow->dst, pstr, cBuf, adv_unit_select_callback); pBuf->data.unit = pUnit; @@ -1990,6 +1999,9 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ vetname = utype_veteran_name_translation(pUnitType, pUnit->veteran); if ((pCity && city_owner(pCity) == client.conn.playing) || (unit_owner(pUnit) == client.conn.playing)) { + struct astring addition = ASTRING_INIT; + + unit_activity_astr(pUnit, &addition); fc_snprintf(cBuf, sizeof(cBuf), _("Activate %s (%d / %d) %s (%d,%d,%s) %s"), (vetname != NULL ? vetname : ""), @@ -1998,7 +2010,8 @@ void popup_advanced_terrain_dialog(struct tile *ptile, Uint16 pos_x, Uint16 pos_ pUnitType->attack_strength, pUnitType->defense_strength, move_points_text(pUnitType->move_rate, FALSE), - unit_activity_text(pUnit)); + astr_str(&addition)); + astr_free(&addition); create_active_iconlabel(pBuf, pWindow->dst, pstr, cBuf, adv_unit_select_callback); diff --git a/client/gui-sdl2/mapview.c b/client/gui-sdl2/mapview.c index 4675b66467..1b6cc1cd83 100644 --- a/client/gui-sdl2/mapview.c +++ b/client/gui-sdl2/mapview.c @@ -801,6 +801,7 @@ void redraw_unit_info_label(struct unit_list *punitlist) unit_list_iterate(pTile->units, aunit) { SDL_Surface *tmp_surf; + struct astring addition = ASTRING_INIT; if (aunit == pUnit) { continue; @@ -809,6 +810,7 @@ void redraw_unit_info_label(struct unit_list *punitlist) pUType = unit_type_get(aunit); vetname = utype_veteran_name_translation(pUType, aunit->veteran); pHome_City = game_city_by_number(aunit->homecity); + unit_activity_astr(aunit, &addition); fc_snprintf(buffer, sizeof(buffer), "%s (%d,%d,%s)%s%s\n%s\n(%d/%d)\n%s", utype_name_translation(pUType), pUType->attack_strength, @@ -816,9 +818,10 @@ void redraw_unit_info_label(struct unit_list *punitlist) move_points_text(pUType->move_rate, FALSE), (vetname != NULL ? "\n" : ""), (vetname != NULL ? vetname : ""), - unit_activity_text(aunit), + astr_str(&addition), aunit->hp, pUType->hp, pHome_City ? city_name_get(pHome_City) : Q_("?homecity:None")); + astr_free(&addition); buf_surf = create_surf(tileset_full_tile_width(tileset), tileset_full_tile_height(tileset), SDL_SWSURFACE); diff --git a/client/gui-xaw/dialogs.c b/client/gui-xaw/dialogs.c index 592e623a5f..402834d683 100644 --- a/client/gui-xaw/dialogs.c +++ b/client/gui-xaw/dialogs.c @@ -33,6 +33,7 @@ #include /* for racesdlg */ /* utility */ +#include "astring.h" #include "bitvector.h" #include "fcintl.h" #include "log.h" @@ -699,6 +700,7 @@ void unit_select_dialog_popup(struct tile *ptile) struct unit_type *punittemp = unit_type_get(punit); struct city *pcity; struct canvas store; + struct astring addition = ASTRING_INIT; if (!(i % r)) { nargs = 0; @@ -718,10 +720,12 @@ void unit_select_dialog_popup(struct tile *ptile) pcity = player_city_by_number(client_player(), punit->homecity); + unit_activity_astr(punit, &addition); fc_snprintf(buffer, sizeof(buffer), "%s(%s)\n%s", utype_name_translation(punittemp), pcity ? city_name_get(pcity) : "", - unit_activity_text(punit)); + astr_str(&addition)); + astr_free(&addition); unit_select_pixmaps[i]=XCreatePixmap(display, XtWindow(map_canvas), tileset_full_tile_width(tileset), tileset_full_tile_height(tileset), diff --git a/client/text.c b/client/text.c index 4967ddd561..21c3446a1a 100644 --- a/client/text.c +++ b/client/text.c @@ -1106,8 +1106,11 @@ const char *get_unit_info_label_text2(struct unit_list *punits, int linebreaks) astr_add_line(&str, _("Turns to target: %d to %d"), min, max); } } else if (count == 1) { - astr_add_line(&str, "%s", - unit_activity_text(unit_list_get(punits, 0))); + struct astring addition = ASTRING_INIT; + + unit_activity_astr(unit_list_get(punits, 0), &addition); + astr_add_line(&str, "%s", astr_str(&addition)); + astr_free(&addition); } else if (count > 1) { astr_add_line(&str, PL_("%d unit selected", "%d units selected", diff --git a/common/unit.c b/common/unit.c index 59d7cd10f6..9feb98af84 100644 --- a/common/unit.c +++ b/common/unit.c @@ -1314,21 +1314,6 @@ bv_extras get_unit_tile_pillage_set(const struct tile *ptile) return tgt_ret; } -/************************************************************************** - Return text describing the unit's current activity as a static string. - - FIXME: Convert all callers of this function to unit_activity_astr() - because this function is not re-entrant. -**************************************************************************/ -const char *unit_activity_text(const struct unit *punit) { - static struct astring str = ASTRING_INIT; - - astr_clear(&str); - unit_activity_astr(punit, &str); - - return astr_str(&str); -} - /************************************************************************** Append text describing the unit's current activity to the given astring. **************************************************************************/ diff --git a/common/unit.h b/common/unit.h index fb37a19e59..9835166228 100644 --- a/common/unit.h +++ b/common/unit.h @@ -325,7 +325,6 @@ bool kills_citizen_after_attack(const struct unit *punit); struct astring; /* Forward declaration. */ void unit_activity_astr(const struct unit *punit, struct astring *astr); void unit_upkeep_astr(const struct unit *punit, struct astring *astr); -const char *unit_activity_text(const struct unit *punit); int get_transporter_capacity(const struct unit *punit); -- 2.39.0