From ec1cf005b6a8702555e97a5f1e6f03bf5f5724dc Mon Sep 17 00:00:00 2001 From: Kp Date: Sun, 26 Aug 2018 18:10:36 +0000 Subject: [PATCH] Enable -Wformat-truncation Add macro cf_assert ("control flow" assert) to hint to gcc that certain conditions are impossible. Use it to avoid generating range checks for situations that never happen. If the event did happen, the only consequence would be truncated UI text, rather than a correctness problem. --- SConstruct | 30 ---------- common/arch/sdl/joy.cpp | 7 ++- common/editor/autosave.cpp | 7 ++- common/include/compiler-cf_assert.h | 86 +++++++++++++++++++++++++++++ common/ui/menubar.cpp | 4 +- d2x-rebirth/main/escort.cpp | 2 +- similar/editor/kmine.cpp | 4 +- similar/main/bmread.cpp | 14 ++--- similar/main/gameseq.cpp | 58 +++++++++---------- similar/main/mission.cpp | 49 +++++++++++----- similar/main/net_udp.cpp | 49 ++++++++++------ similar/main/newdemo.cpp | 4 +- similar/main/piggy.cpp | 2 + 13 files changed, 211 insertions(+), 105 deletions(-) create mode 100644 common/include/compiler-cf_assert.h diff --git a/SConstruct b/SConstruct index 0e8d520f1..edd09b3d9 100644 --- a/SConstruct +++ b/SConstruct @@ -2576,36 +2576,6 @@ where the cast is useless. '-Wsuggest-attribute=noreturn', '-Wlogical-op', '-Wold-style-cast', - # Starting in gcc-7, Rebirth default options cause gcc to enable - # -Wformat-truncation automatically. Unless proven otherwise by - # data flow analysis, gcc pessimistically assumes that input - # parameters might have their most space-consuming value (3 - # digits for a uint8_t, 5 for uint16_t, etc.). This causes - # numerous warnings for places where Rebirth allocated a buffer - # that is exactly big enough for the small numbers that are - # actually used, but the data flow analysis is unable to prove - # that larger numbers are not used. - # - # It would be nice to remove this option and eliminate the - # warnings with fixes in the code, since this test completely - # suppresses all -Wformat-truncation diagnostics, including any - # that may be true bugs. However, gcc provides no documented - # way to do this that does not generate extra runtime - # instructions, which are unnecessary in at least some of the - # cases where gcc warns. - # - # In testing, setting -Wformat-truncation=1 was insufficient to - # silence a warning in similar/main/net_udp.cpp: - # - # similar/main/net_udp.cpp: In static member function 'static void {anonymous}::more_game_options_menu_items::net_udp_more_game_options()': - # similar/main/net_udp.cpp:3528:6: error: ' Furthest Sites' directive output may be truncated writing 15 bytes into a region of size between 14 and 16 [-Werror=format-truncation=] - # void more_game_options_menu_items::net_udp_more_game_options() - # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ - # similar/main/net_udp.cpp:3433:11: note: 'snprintf' output between 21 and 23 bytes into a destination of size 21 - # snprintf(SecludedSpawnText, sizeof(SecludedSpawnText), "Use %u Furthest Sites", Netgame.SecludedSpawns + 1); - # ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - # - '-Wno-format-truncation', ] __preferred_win32_linker_options = [ '-Wl,--large-address-aware', diff --git a/common/arch/sdl/joy.cpp b/common/arch/sdl/joy.cpp index 4bcc9034c..bebe92e33 100644 --- a/common/arch/sdl/joy.cpp +++ b/common/arch/sdl/joy.cpp @@ -24,6 +24,7 @@ #include "compiler-range_for.h" #if DXX_MAX_JOYSTICKS +#include "compiler-cf_assert.h" #include "compiler-integer_sequence.h" #include "d_enumerate.h" #include "partial_range.h" @@ -294,7 +295,8 @@ void joy_init() const auto n = check_warn_joy_support_limit(SDL_NumJoysticks(), "joystick", DXX_MAX_JOYSTICKS); unsigned joystick_n_buttons = 0, joystick_n_axes = 0; - for (int i = 0; i < n; i++) { + for (unsigned i = 0; i < n; i++) + { auto &joystick = SDL_Joysticks[num_joysticks]; const auto handle = SDL_JoystickOpen(i); joystick.handle().reset(handle); @@ -311,6 +313,7 @@ void joy_init() joyaxis_text.resize(joyaxis_text.size() + n_axes); range_for (auto &&e, enumerate(partial_range(joystick.axis_map(), n_axes), 1)) { + cf_assert(e.idx <= DXX_MAX_AXES_PER_JOYSTICK); auto &text = joyaxis_text[joystick_n_axes]; e.value = joystick_n_axes++; snprintf(&text[0], sizeof(text), "J%d A%u", i + 1, e.idx); @@ -325,6 +328,7 @@ void joy_init() #if DXX_MAX_BUTTONS_PER_JOYSTICK range_for (auto &&e, enumerate(partial_range(joystick.button_map(), n_buttons), 1)) { + cf_assert(e.idx <= DXX_MAX_BUTTONS_PER_JOYSTICK); auto &text = joybutton_text[joystick_n_buttons]; e.value = joystick_n_buttons++; snprintf(&text[0], sizeof(text), "J%d B%d", i + 1, e.idx); @@ -334,6 +338,7 @@ void joy_init() range_for (auto &&e, enumerate(partial_range(joystick.hat_map(), n_hats), 1)) { e.value = joystick_n_buttons; + cf_assert(e.idx <= DXX_MAX_HATS_PER_JOYSTICK); //a hat counts as four buttons snprintf(&joybutton_text[joystick_n_buttons++][0], sizeof(joybutton_text[0]), "J%d H%d%c", i + 1, e.idx, 0202); snprintf(&joybutton_text[joystick_n_buttons++][0], sizeof(joybutton_text[0]), "J%d H%d%c", i + 1, e.idx, 0177); diff --git a/common/editor/autosave.cpp b/common/editor/autosave.cpp index 0d286e8ce..d31719365 100644 --- a/common/editor/autosave.cpp +++ b/common/editor/autosave.cpp @@ -39,6 +39,8 @@ COPYRIGHT 1993-1998 PARALLAX SOFTWARE CORPORATION. ALL RIGHTS RESERVED. #include "ui.h" #include "strutil.h" +#include "compiler-cf_assert.h" + namespace dcx { #define AUTOSAVE_PERIOD 5 // Number of minutes for timed autosave @@ -66,7 +68,10 @@ void init_autosave(void) { void close_autosave(void) { char *ext; - for (int i=0;i. + * It is copyright by its individual contributors, as recorded in the + * project's Git history. See COPYING.txt at the top level for license + * terms and a link to the Git history. + */ +#pragma once + +/* Define a utility macro `cf_assert` that, like `assert`, is meant to + * check invariants. Unlike `assert`, it has a small but configurable + * effect on NDEBUG builds. + * + * This macro is mainly used to try to hint to gcc that it is + * excessively cautious in determining whether a + * -Wformat-truncation warning is appropriate. + * + * Unless proved otherwise by control flow analysis, gcc will assume + * that a variable to be formatted could have any value that fits in the + * underlying type, and then gcc will warn if any of those values does + * not fit. In most cases, program logic constrains the value to a + * smaller range than the underlying type supports. Correct placement + * of this macro can inform the compiler that a variable's actual range + * is less than the full supported range of the underlying type. For + * example, a player counter should never exceed MAX_PLAYERS, but + * MAX_PLAYERS is far less than MAX_UCHAR. + * + * Unfortunately, in tested versions of gcc-8, the compiler's range + * propagation pass is hampered by strange rules in the flow control + * logic. Consider: + + unsigned var = unconstrained_expression(); + // var can now be anything in [0, UINT_MAX] + cf_assert(var <= 8); + // If execution gets here, `var <= 8` is true. + snprintf(..., var); + + * Suppose cf_assert(X) is defined as `((X) || (assert(X), + * __builtin_unreachable()))`, so the above becomes: + + unsigned var = unconstrained_expression(); + if (var <= 8) { + } + else { + assert_fail(...); + __builtin_unreachable(); + } + snprintf(..., var); + + * In testing, gcc deleted __builtin_unreachable (probably because + * assert_fail is noreturn), then warned because, without the + * __builtin_unreachable, flow control decides the snprintf is reachable + * (even though assert_fail is noreturn) on the else path and would + * misbehave if reached. Remove the call to `assert` so that you have + * `else { __builtin_unreachable(); }` and gcc will retain the + * `__builtin_unreachable` and not warn. + * + * For an even more bizarre result: + + if (var <= 8) { + snprintf(..., var); + } else { + assert(var <= 8); // always fails + __builtin_unreachable(); + } + + * This block warns that `var` is out of range. Comment out the + * `assert`, which cannot influence whether `snprintf` is reached, and + * the warning goes away. + * + * -- + * + * Leave cf_assert set as an alias for (X || __builtin_unreachable()) + * unless you know what you are doing and are prepared for the warnings + * that will arise. + */ +#include +#if defined(DXX_CF_ASSERT_ASSERT) +#define cf_assert assert +#else +#ifdef DXX_CF_ASSERT_TRAP +#define cf_assert_fail __builtin_trap +#else +#define cf_assert_fail __builtin_unreachable +#endif +#define cf_assert(X) ((X) ? static_cast(0) : (cf_assert_fail())) +#endif diff --git a/common/ui/menubar.cpp b/common/ui/menubar.cpp index 9c5415125..d7779a193 100644 --- a/common/ui/menubar.cpp +++ b/common/ui/menubar.cpp @@ -741,7 +741,7 @@ int menubar_init(grs_canvas &canvas, const char *const file) { int np; char buf1[200]; - char buf2[200]; + char buf2[204]; num_menus = state = 0; @@ -786,7 +786,7 @@ int menubar_init(grs_canvas &canvas, const char *const file) CommaParse( 2, buf1, buffer ); ul_xlate(buf1); - item.Text.reset(d_strdup(buf1[0] == '-' ? buf1 : (snprintf(buf2, sizeof(buf2), " %s ", buf1), buf2))); + item.Text.reset(d_strdup(buf1[0] == '-' ? buf1 : (snprintf(buf2, sizeof(buf2), " %.197s ", buf1), buf2))); item.InactiveText.reset(d_strdup(item.Text.get())); diff --git a/d2x-rebirth/main/escort.cpp b/d2x-rebirth/main/escort.cpp index 316091ddb..90783c16f 100644 --- a/d2x-rebirth/main/escort.cpp +++ b/d2x-rebirth/main/escort.cpp @@ -1741,7 +1741,7 @@ window_event_result escort_menu::event_handler(window *, const d_event &event, e void do_escort_menu(void) { int next_goal; - char goal_str[32]; + char goal_str[12]; const char *goal_txt; const char *tstr; escort_menu *menu; diff --git a/similar/editor/kmine.cpp b/similar/editor/kmine.cpp index c5936c6f2..db82bff98 100644 --- a/similar/editor/kmine.cpp +++ b/similar/editor/kmine.cpp @@ -178,9 +178,9 @@ static int med_save_situation(char * filename) { auto SaveFile = PHYSFSX_openWriteBuffered(filename); if (!SaveFile) { - char ErrorMessage[200]; + char ErrorMessage[512]; - snprintf(ErrorMessage, sizeof(ErrorMessage), "ERROR: Unable to open %s\n", filename); + snprintf(ErrorMessage, sizeof(ErrorMessage), "ERROR: Unable to open %.480s", filename); ui_messagebox( -2, -2, 1, ErrorMessage, "Ok" ); return 1; } diff --git a/similar/main/bmread.cpp b/similar/main/bmread.cpp index 0fbe7db28..e2e6b2aaa 100644 --- a/similar/main/bmread.cpp +++ b/similar/main/bmread.cpp @@ -267,7 +267,7 @@ static void ab_load(int skip, const char * filename, array tempname; if (skip) { Assert( bogus_bitmap_initialized != 0 ); @@ -293,11 +293,11 @@ static void ab_load(int skip, const char * filename, arrayavg_color = compute_average_pixel(bm[i].get()); - bmp[i] = piggy_register_bitmap(*bm[i].get(), tempname, 0); + bmp[i] = piggy_register_bitmap(*bm[i].get(), tempname.data(), 0); } } diff --git a/similar/main/gameseq.cpp b/similar/main/gameseq.cpp index dcb3cc380..828061029 100644 --- a/similar/main/gameseq.cpp +++ b/similar/main/gameseq.cpp @@ -893,16 +893,11 @@ void StartNewGame(int start_level) static void DoEndLevelScoreGlitz() { int level_points, skill_points, energy_points, shield_points, hostage_points; - int all_hostage_points; - int endgame_points; - char all_hostage_text[64]; - char endgame_text[64]; #define N_GLITZITEMS 9 - char m_str[N_GLITZITEMS][30]; + char m_str[N_GLITZITEMS][32]; newmenu_item m[N_GLITZITEMS]; int i,c; char title[128]; - int is_last_level; #if defined(DXX_BUILD_DESCENT_I) gr_palette_load( gr_palette ); #elif defined(DXX_BUILD_DESCENT_II) @@ -947,26 +942,7 @@ static void DoEndLevelScoreGlitz() hostage_points = 0; } - all_hostage_text[0] = 0; - endgame_text[0] = 0; - auto &plr = get_local_player(); - if (!cheats.enabled && player_info.mission.hostages_on_board == plr.hostages_level) - { - all_hostage_points = player_info.mission.hostages_on_board * 1000 * (Difficulty_level+1); - snprintf(all_hostage_text, sizeof(all_hostage_text), "%s%i\n", TXT_FULL_RESCUE_BONUS, all_hostage_points); - } else - all_hostage_points = 0; - - if (!cheats.enabled && !(Game_mode & GM_MULTI) && plr.lives && Current_level_num == Last_level) - { //player has finished the game! - endgame_points = plr.lives * 10000; - snprintf(endgame_text, sizeof(endgame_text), "%s%i\n", TXT_SHIP_BONUS, endgame_points); - is_last_level=1; - } else - endgame_points = is_last_level = 0; - - add_bonus_points_to_score(player_info, skill_points + energy_points + shield_points + hostage_points + all_hostage_points + endgame_points); c = 0; snprintf(m_str[c++], sizeof(m_str[0]), "%s%i", TXT_SHIELD_BONUS, shield_points); // Return at start to lower menu... @@ -974,9 +950,35 @@ static void DoEndLevelScoreGlitz() snprintf(m_str[c++], sizeof(m_str[0]), "%s%i", TXT_HOSTAGE_BONUS, hostage_points); snprintf(m_str[c++], sizeof(m_str[0]), "%s%i", TXT_SKILL_BONUS, skill_points); - snprintf(m_str[c++], sizeof(m_str[0]), "%s", all_hostage_text); - if (!(Game_mode & GM_MULTI) && plr.lives && Current_level_num == Last_level) - snprintf(m_str[c++], sizeof(m_str[0]), "%s", endgame_text); + const unsigned hostages_on_board = player_info.mission.hostages_on_board; + unsigned all_hostage_points = 0; + unsigned endgame_points = 0; + uint8_t is_last_level = 0; + auto &hostage_text = m_str[c++]; + if (cheats.enabled) + snprintf(hostage_text, sizeof(hostage_text), "Hostages saved: \t%u", hostages_on_board); + else if (const auto hostages_lost = plr.hostages_level - hostages_on_board) + snprintf(hostage_text, sizeof(hostage_text), "Hostages lost: \t%u", hostages_lost); + else + { + all_hostage_points = hostages_on_board * 1000 * (Difficulty_level + 1); + snprintf(hostage_text, sizeof(hostage_text), "%s%i\n", TXT_FULL_RESCUE_BONUS, all_hostage_points); + } + + auto &endgame_text = m_str[c++]; + endgame_text[0] = 0; + if (cheats.enabled) + { + /* Nothing */ + } + else if (!(Game_mode & GM_MULTI) && plr.lives && Current_level_num == Last_level) + { //player has finished the game! + endgame_points = plr.lives * 10000; + snprintf(endgame_text, sizeof(endgame_text), "%s%i\n", TXT_SHIP_BONUS, endgame_points); + is_last_level=1; + } + + add_bonus_points_to_score(player_info, skill_points + energy_points + shield_points + hostage_points + all_hostage_points + endgame_points); snprintf(m_str[c++], sizeof(m_str[0]), "%s%i\n", TXT_TOTAL_BONUS, shield_points + energy_points + hostage_points + skill_points + all_hostage_points + endgame_points); snprintf(m_str[c++], sizeof(m_str[0]), "%s%i", TXT_TOTAL_SCORE, player_info.mission.score); diff --git a/similar/main/mission.cpp b/similar/main/mission.cpp index f08fcb34d..0c7e3b8e3 100644 --- a/similar/main/mission.cpp +++ b/similar/main/mission.cpp @@ -216,10 +216,13 @@ static const char *load_mission_d1() break; case D1_OEM_MISSION_HOGSIZE: case D1_OEM_10_MISSION_HOGSIZE: + { N_secret_levels = 1; - Last_level = 15; - Last_secret_level = -1; + constexpr unsigned last_level = 15; + constexpr int last_secret_level = -1; + Last_level = last_level; + Last_secret_level = last_secret_level; if (!allocate_levels()) { @@ -228,14 +231,24 @@ static const char *load_mission_d1() } //build level names - for (int i=0; i < Last_level - 1; i++) - snprintf(&Level_names[i][0u], Level_names[i].size(), "level%02d.rdl", i+1); - snprintf(&Level_names[Last_level - 1][0u], Level_names[Last_level - 1].size(), "saturn%02d.rdl", Last_level); - for (int i=0; i < -Last_secret_level; i++) - snprintf(&Secret_level_names[i][0u], Secret_level_names[i].size(), "levels%1d.rdl", i+1); + for (unsigned i = 0; i < last_level - 1; ++i) + { + auto &ln = Level_names[i]; + snprintf(&ln[0u], ln.size(), "level%02u.rdl", i + 1); + } + { + auto &ln = Level_names[last_level - 1]; + snprintf(&ln[0u], ln.size(), "saturn%02d.rdl", last_level); + } + for (int i = 0; i < -last_secret_level; ++i) + { + auto &sn = Secret_level_names[i]; + snprintf(&sn[0u], sn.size(), "levels%1d.rdl", i + 1); + } Secret_level_table[0] = 10; Briefing_text_filename = "briefsat.txb"; Ending_text_filename = BIMD1_ENDING_FILE_OEM; + } break; default: Int3(); // fall through @@ -243,10 +256,13 @@ static const char *load_mission_d1() case D1_MISSION_HOGSIZE2: case D1_10_MISSION_HOGSIZE: case D1_MAC_MISSION_HOGSIZE: + { N_secret_levels = 3; - Last_level = BIMD1_LAST_LEVEL; - Last_secret_level = BIMD1_LAST_SECRET_LEVEL; + constexpr unsigned last_level = BIMD1_LAST_LEVEL; + constexpr int last_secret_level = BIMD1_LAST_SECRET_LEVEL; + Last_level = last_level; + Last_secret_level = last_secret_level; if (!allocate_levels()) { @@ -255,16 +271,23 @@ static const char *load_mission_d1() } //build level names - for (int i=0;i m; - array, entries> ljtext; + array, entries> ljtext; }; manual_join_user_inputs manual_join::s_last_inputs; @@ -1206,7 +1207,8 @@ static int net_udp_list_join_poll(newmenu *menu, const d_event &event, list_join // Copy the active games data into the menu options for (int i = 0; i < UDP_NETGAMES_PPAGE; i++) { - int game_status = Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].game_status; + const auto &augi = Active_udp_games[(i + (NLPage * UDP_NETGAMES_PPAGE))]; + int game_status = augi.game_status; int j,x, k,nplayers = 0; char levelname[8],MissName[25],GameName[25],thold[2]; thold[1]=0; @@ -1224,9 +1226,9 @@ static int net_udp_list_join_poll(newmenu *menu, const d_event &event, list_join const auto &&fspacx = FSPACX(); for (x=0,k=0,j=0;j<15;j++) { - if (Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].mission_title[j]=='\t') + if (augi.mission_title[j]=='\t') continue; - thold[0]=Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].mission_title[j]; + thold[0] = augi.mission_title[j]; int tx; gr_get_string_size(*grd_curcanv->cv_font, thold, &tx, nullptr, nullptr); @@ -1237,15 +1239,15 @@ static int net_udp_list_join_poll(newmenu *menu, const d_event &event, list_join break; } - MissName[k++]=Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].mission_title[j]; + MissName[k++] = augi.mission_title[j]; } MissName[k]=0; for (x=0,k=0,j=0;j<15;j++) { - if (Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].game_name[j]=='\t') + if (augi.game_name[j]=='\t') continue; - thold[0]=Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].game_name[j]; + thold[0]=augi.game_name[j]; int tx; gr_get_string_size(*grd_curcanv->cv_font, thold, &tx, nullptr, nullptr); @@ -1255,25 +1257,32 @@ static int net_udp_list_join_poll(newmenu *menu, const d_event &event, list_join k+=3; break; } - GameName[k++]=Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].game_name[j]; + GameName[k++]=augi.game_name[j]; } GameName[k]=0; - nplayers = Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].numconnected; + nplayers = augi.numconnected; - if (Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].levelnum < 0) - snprintf(levelname, sizeof(levelname), "S%d", -Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].levelnum); + const int levelnum = augi.levelnum; + if (levelnum < 0) + { + cf_assert(-levelnum < MAX_SECRET_LEVELS_PER_MISSION); + snprintf(levelname, sizeof(levelname), "S%d", -levelnum); + } else - snprintf(levelname, sizeof(levelname), "%d", Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].levelnum); + { + cf_assert(levelnum < MAX_LEVELS_PER_MISSION); + snprintf(levelname, sizeof(levelname), "%d", levelnum); + } const char *status; if (game_status == NETSTAT_STARTING) status = "FORMING "; else if (game_status == NETSTAT_PLAYING) { - if (Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].RefusePlayers) + if (augi.RefusePlayers) status = "RESTRICT"; - else if (Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].game_flag.closed) + else if (augi.game_flag.closed) status = "CLOSED "; else status = "OPEN "; @@ -1281,9 +1290,9 @@ static int net_udp_list_join_poll(newmenu *menu, const d_event &event, list_join else status = "BETWEEN "; - unsigned gamemode = Active_udp_games[(i+(NLPage*UDP_NETGAMES_PPAGE))].gamemode; + unsigned gamemode = augi.gamemode; auto &p = dj->ljtext[i]; - snprintf(&p[0], p.size(), "%d.\t%s \t%s \t %d/%d \t%s \t %s \t%s", (i + (NLPage * UDP_NETGAMES_PPAGE)) + 1, GameName, (gamemode < sizeof(GMNamesShrt) / sizeof(GMNamesShrt[0])) ? GMNamesShrt[gamemode] : "INVALID", nplayers, Active_udp_games[(i + (NLPage * UDP_NETGAMES_PPAGE))].max_numplayers, MissName, levelname, status); + snprintf(&p[0], p.size(), "%d.\t%.24s \t%.7s \t%3u/%u \t%.24s \t %s \t%s", (i + (NLPage * UDP_NETGAMES_PPAGE)) + 1, GameName, (gamemode < sizeof(GMNamesShrt) / sizeof(GMNamesShrt[0])) ? GMNamesShrt[gamemode] : "INVALID", nplayers, augi.max_numplayers, MissName, levelname, status); } return 0; } @@ -3464,7 +3473,9 @@ public: } void update_secluded_spawn_string() { - snprintf(SecludedSpawnText, sizeof(SecludedSpawnText), "Use %u Furthest Sites", Netgame.SecludedSpawns + 1); + const unsigned SecludedSpawns = Netgame.SecludedSpawns; + cf_assert(SecludedSpawns < MAX_PLAYERS); + snprintf(SecludedSpawnText, sizeof(SecludedSpawnText), "Use %u Furthest Sites", SecludedSpawns + 1); } void update_kill_goal_string() { @@ -3712,7 +3723,9 @@ struct param_opt } void update_max_players_string() { - snprintf(srmaxnet, sizeof(srmaxnet), "Maximum players: %u", Netgame.max_numplayers); + const unsigned max_numplayers = Netgame.max_numplayers; + cf_assert(max_numplayers < MAX_PLAYERS); + snprintf(srmaxnet, sizeof(srmaxnet), "Maximum players: %u", max_numplayers); } }; diff --git a/similar/main/newdemo.cpp b/similar/main/newdemo.cpp index f5e305bd0..583c6411a 100644 --- a/similar/main/newdemo.cpp +++ b/similar/main/newdemo.cpp @@ -3876,7 +3876,7 @@ static void newdemo_write_end() nd_write_byte(ND_EVENT_EOF); } -static bool guess_demo_name(ntstring &filename) +static bool guess_demo_name(ntstring &filename) { filename[0] = 0; const auto &n = CGameArg.SysRecordDemoNameTemplate; @@ -3964,7 +3964,7 @@ void newdemo_stop_recording() { int exit; static sbyte tmpcnt = 0; - ntstring filename; + ntstring filename; exit = 0; diff --git a/similar/main/piggy.cpp b/similar/main/piggy.cpp index 00e0dbf97..762957305 100644 --- a/similar/main/piggy.cpp +++ b/similar/main/piggy.cpp @@ -620,6 +620,7 @@ int properties_init() return retval; } #elif defined(DXX_BUILD_DESCENT_II) + //initialize a pigfile, reading headers //returns the size of all the bitmap data void piggy_init_pigfile(const char *filename) @@ -819,6 +820,7 @@ void piggy_new_pigfile(char *pigname) unsigned nframes; const std::size_t len = p - AllBitmaps[i].name.data(); + cf_assert(len < AllBitmaps[i].name.size()); memcpy(basename, AllBitmaps[i].name.data(), len); basename[len] = 0;