From 482c5a5cc047a7e941e8ef6ff78e1b34a6e04c58 Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sat, 28 Feb 2026 15:50:06 -0500 Subject: [PATCH] fix some waypoint bugs 1. The `waypoint_stuff_name` function used the wrong length when building the waypoint name, causing the number to be written past the end of the string in most cases. Not sure how this escaped testing. 2. Change two Assertions, which had no effect due to the comma operator, to Errors, which match their counterparts in their respective functions. 3. Make `internal_error` consistent between the campaign editor, the mission editor, and qtFRED, and prune an unnecessary string copy. Follow-up to #7104. Fixes #7254. --- code/object/waypoint.cpp | 9 +++++---- fred2/campaigntreewnd.cpp | 17 +++++++++++------ fred2/fredview.cpp | 15 +++++---------- qtfred/src/mission/Editor.cpp | 13 ++++--------- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/code/object/waypoint.cpp b/code/object/waypoint.cpp index 955d29deb2c..ba6df05573c 100644 --- a/code/object/waypoint.cpp +++ b/code/object/waypoint.cpp @@ -399,7 +399,7 @@ void waypoint_stuff_name(char *dest, const char *waypoint_list_name, int waypoin if (waypoint_num < 1) { - Assertion(LOCATION, "A waypoint number must be at least 1!"); + Error(LOCATION, "A waypoint number must be at least 1!"); *dest = 0; return; } @@ -410,8 +410,9 @@ void waypoint_stuff_name(char *dest, const char *waypoint_list_name, int waypoin return; } - strncpy(dest, waypoint_list_name, name_max_len); - sprintf(dest + name_max_len, ":%d", waypoint_num); + auto name_len = std::min(strlen(waypoint_list_name), name_max_len); + strncpy(dest, waypoint_list_name, name_len); + sprintf(dest + name_len, ":%d", waypoint_num); } void waypoint_stuff_name(SCP_string &dest, const char *waypoint_list_name, int waypoint_num) @@ -420,7 +421,7 @@ void waypoint_stuff_name(SCP_string &dest, const char *waypoint_list_name, int w if (waypoint_num < 1) { - Assertion(LOCATION, "A waypoint number must be at least 1!"); + Error(LOCATION, "A waypoint number must be at least 1!"); dest = ""; return; } diff --git a/fred2/campaigntreewnd.cpp b/fred2/campaigntreewnd.cpp index 527fcf37e50..680fad3928d 100644 --- a/fred2/campaigntreewnd.cpp +++ b/fred2/campaigntreewnd.cpp @@ -480,20 +480,25 @@ int campaign_tree_wnd::error(const char *msg, ...) int campaign_tree_wnd::internal_error(const char *msg, ...) { - SCP_string buf, buf2; + SCP_string buf; va_list args; - g_err++; va_start(args, msg); vsprintf(buf, msg, args); va_end(args); - sprintf(buf2, "%s\n\nThis is an internal error. Please let Hoffoss\n" - "know about this so he can fix it. Click cancel to debug.", buf.c_str()); + g_err++; - nprintf(("Error", buf.c_str())); - if (MessageBox(buf2.c_str(), "Internal Error", MB_OKCANCEL | MB_ICONEXCLAMATION) == IDCANCEL) +#ifndef NDEBUG + nprintf(("Internal Error", buf.c_str())); + + buf += "\n\nThis is an internal error. Please notify a coder about this. Click cancel to debug."; + + if (MessageBox(buf.c_str(), "Internal Error", MB_OKCANCEL | MB_ICONEXCLAMATION) == IDCANCEL) Int3(); // drop to debugger so the problem can be analyzed. +#else + MessageBox(buf.c_str(), "Error", MB_OK | MB_ICONEXCLAMATION); +#endif return -1; } diff --git a/fred2/fredview.cpp b/fred2/fredview.cpp index fba2463281b..b610ed08175 100644 --- a/fred2/fredview.cpp +++ b/fred2/fredview.cpp @@ -3400,27 +3400,22 @@ int CFREDView::error(const char *msg, ...) int CFREDView::internal_error(const char *msg, ...) { - char buf[2048]; + SCP_string buf; va_list args; va_start(args, msg); - vsnprintf(buf, sizeof(buf)-1, msg, args); + vsprintf(buf, msg, args); va_end(args); - buf[sizeof(buf)-1] = '\0'; g_err = 1; #ifndef NDEBUG - char buf2[2048]; + buf += "\n\nThis is an internal error. Please notify a coder about this. Click cancel to debug."; - sprintf(buf2, "%s\n\nThis is an internal error. Please let Jason\n" - "know about this so he can fix it. Click cancel to debug.", buf); - - if (MessageBox(buf2, "Internal Error", MB_OKCANCEL | MB_ICONEXCLAMATION) == IDCANCEL) + if (MessageBox(buf.c_str(), "Internal Error", MB_OKCANCEL | MB_ICONEXCLAMATION) == IDCANCEL) Int3(); // drop to debugger so the problem can be analyzed. - #else - MessageBox(buf, "Error", MB_OK | MB_ICONEXCLAMATION); + MessageBox(buf.c_str(), "Error", MB_OK | MB_ICONEXCLAMATION); #endif return -1; diff --git a/qtfred/src/mission/Editor.cpp b/qtfred/src/mission/Editor.cpp index 21bde4cabc8..93f77e1e852 100644 --- a/qtfred/src/mission/Editor.cpp +++ b/qtfred/src/mission/Editor.cpp @@ -2642,29 +2642,24 @@ int Editor::error(const char* msg, ...) { return 1; } int Editor::internal_error(const char* msg, ...) { - char buf[2048]; + SCP_string buf; va_list args; va_start(args, msg); - vsnprintf(buf, sizeof(buf) - 1, msg, args); + vsprintf(buf, msg, args); va_end(args); - buf[sizeof(buf) - 1] = '\0'; g_err = 1; #ifndef NDEBUG - char buf2[2048]; - - sprintf_safe(buf2, "%s\n\nThis is an internal error. Please let Jason\n" - "know about this so he can fix it. Click cancel to debug.", buf); + buf += "\n\nThis is an internal error. Please notify a coder about this. Click cancel to debug."; if (_lastActiveViewport->dialogProvider->showButtonDialog(DialogType::Error, "Internal Error", - buf2, + buf, { DialogButton::Ok, DialogButton::Cancel }) == DialogButton::Cancel) Int3(); // drop to debugger so the problem can be analyzed. - #else _lastActiveViewport->dialogProvider->showButtonDialog(DialogType::Error, "Error", buf, { DialogButton::Ok }); #endif