add move semantics for message media references (scp-fs2open#7517)

MMessage's avi_info/wave_info unions hold strdup'd filenames in FRED
(indices in the game), but moves copied the pointers without nulling
the source, so every vector shift on Messages left moved-from elements
aliasing live allocations -- one careless edit away from a double-free.

Wrap each union in a MessageMediaRef struct whose moves null the source
and whose copies stay shallow (the event editor's backup pattern copies
then re-strdups).  MMessage keeps its implicit copy and gains a correct
implicit move; access syntax at all use sites is unchanged.

Add a message_free_media_names() helper and use it at the FRED
delete/wipe sites.  Two fixes ride along: messages_init() now frees
mission messages' media names before discarding them, closing a leak
that fired on every mission load in FRED; and the message editor's
OnDelete now uses the move-based array_remove_slot instead of a
copy-shift that left a stale aliasing tail.

No destructor: the active union member depends on Fred_running, and
shallow copies alias, so freeing remains explicit.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: Goober5000

code/mission/missionmessage.cpp

@@ -974,11 +974,34 @@ void messages_init()
 	Next_mute_time = 1;
 
 	//wipe all the non-builtin messages
-	Messages.erase((Messages.begin()+Num_builtin_messages), Messages.end()); 
-	Message_avis.erase((Message_avis.begin()+Num_builtin_avis), Message_avis.end()); 
+	if (Fred_running) {
+		// in FRED the media unions hold strdup'd names which must be freed before the messages are discarded
+		for (auto it = Messages.begin() + Num_builtin_messages; it != Messages.end(); ++it)
+			message_free_media_names(*it);
+	}
+	Messages.erase((Messages.begin()+Num_builtin_messages), Messages.end());
+	Message_avis.erase((Message_avis.begin()+Num_builtin_avis), Message_avis.end());
 	Message_waves.erase((Message_waves.begin()+Num_builtin_waves), Message_waves.end());
 }
 
+// FRED stores strdup'd filenames in the message media unions (the game
+// stores indices, which must not be freed).  Frees and nulls both names.
+void message_free_media_names(MMessage &msg)
+{
+	Assertion(Fred_running, "message_free_media_names is only valid in FRED, where the media unions hold name pointers!");
+
+	if (msg.avi_info.name)
+	{
+		free(msg.avi_info.name);
+		msg.avi_info.name = nullptr;
+	}
+	if (msg.wave_info.name)
+	{
+		free(msg.wave_info.name);
+		msg.wave_info.name = nullptr;
+	}
+}
+
 // free a loaded avi
 void message_mission_free_avi(int m_index)
 {

code/mission/missionmessage.h

@@ -167,6 +167,35 @@ typedef struct MessageFilter {
 	int                    team_bitfield;
 } MessageFilter;
 
+// Media reference for a message: the game stores an index into the avi or
+// wave arrays, FRED stores a strdup'd filename.  Implements move operations.
+// No destructor; freeing remains the responsibility of the free() sites.
+struct MessageMediaRef
+{
+	union
+	{
+		int   index;
+		char *name;
+	};
+
+	MessageMediaRef() : name(nullptr) {}
+
+	MessageMediaRef(const MessageMediaRef &) = default;
+	MessageMediaRef &operator=(const MessageMediaRef &) = default;
+
+	MessageMediaRef(MessageMediaRef &&other) noexcept : name(other.name)
+	{
+		other.name = nullptr;
+	}
+
+	MessageMediaRef &operator=(MessageMediaRef &&other) noexcept
+	{
+		name = other.name;
+		other.name = nullptr;
+		return *this;
+	}
+};
+
 typedef struct MissionMessage {
 	char    name[NAME_LENGTH];					// used to identify this message
 	char    message[MESSAGE_LENGTH];			// actual message
@@ -182,22 +211,17 @@ typedef struct MissionMessage {
 	int outer_filter_radius;
 	int boost_level;
 
-	// unions for avi/wave information.  Because of issues with Fred, we are using
+	// avi/wave information.  Because of issues with Fred, we are using
 	// the union to specify either the index into the avi or wave arrays above,
-	// or refernce the name directly.  The currently plan is to only have Fred reference
-	// the name field!!!
-	union {
-		int	index;								// index of avi file to play
-		char	*name;
-	} avi_info;
-
-	union {
-		int	index;
-		char	*name;
-	} wave_info;
+	// or reference the name directly.  The current plan is to only have Fred
+	// reference the name field!!!
+	MessageMediaRef avi_info;
+	MessageMediaRef wave_info;
 
 } MMessage;
 
+extern void message_free_media_names(MMessage &msg);
+
 extern SCP_vector<MMessage> Messages;
 
 typedef struct pmessage {

fred2/eventeditor.cpp

@@ -570,13 +570,8 @@ void event_editor::OnButtonOk()
 	for (const auto &name_pair: names)
 		update_sexp_references(name_pair.first.c_str(), name_pair.second.c_str(), OPF_EVENT_NAME);
 
-	for (i=Num_builtin_messages; i<Num_messages; i++) {
-		if (Messages[i].avi_info.name)
-			free(Messages[i].avi_info.name);
-
-		if (Messages[i].wave_info.name)
-			free(Messages[i].wave_info.name);
-	}
+	for (i=Num_builtin_messages; i<Num_messages; i++)
+		message_free_media_names(Messages[i]);
 
 	Num_messages = (int)m_messages.size() + Num_builtin_messages;
 	Messages.resize(Num_messages);

fred2/messageeditordlg.cpp

@@ -447,26 +447,23 @@ int CMessageEditorDlg::update(int num)
 	return 0;
 }
 
-void CMessageEditorDlg::OnDelete() 
+void CMessageEditorDlg::OnDelete()
 {
 	char buf[256];
 	int i;
 
 	Assert((m_cur_msg >= 0) && (m_cur_msg < Num_messages));
-	if (Messages[m_cur_msg].avi_info.name)
-		free(Messages[m_cur_msg].avi_info.name);
-	if (Messages[m_cur_msg].wave_info.name)
-		free(Messages[m_cur_msg].wave_info.name);
+	message_free_media_names(Messages[m_cur_msg]);
 
 	((CListBox *) GetDlgItem(IDC_MESSAGE_LIST))->DeleteString(m_cur_msg);
 	sprintf(buf, "<%s>", Messages[m_cur_msg].name);
 	update_sexp_references(Messages[m_cur_msg].name, buf, OPF_MESSAGE);
 	update_sexp_references(Messages[m_cur_msg].name, buf, OPF_MESSAGE_OR_STRING);
 
 	for (i=m_cur_msg; i<Num_messages-1; i++)
-		Messages[i] = Messages[i + 1];
-
+		Messages[i] = std::move(Messages[i + 1]);
 	Num_messages--;
+
 	if (m_cur_msg >= Num_messages)
 		m_cur_msg = Num_messages - 1;
 

qtfred/src/mission/dialogs/MissionEventsDialogModel.cpp

@@ -78,13 +78,8 @@ bool MissionEventsDialogModel::apply()
 	for (const auto& name_pair : names)
 		update_sexp_references(name_pair.first.c_str(), name_pair.second.c_str(), OPF_EVENT_NAME);
 
-	for (int i = Num_builtin_messages; i < Num_messages; i++) {
-		if (Messages[i].avi_info.name)
-			free(Messages[i].avi_info.name);
-
-		if (Messages[i].wave_info.name)
-			free(Messages[i].wave_info.name);
-	}
+	for (int i = Num_builtin_messages; i < Num_messages; i++)
+		message_free_media_names(Messages[i]);
 
 	Num_messages = static_cast<int>(m_messages.size()) + Num_builtin_messages;
 	Messages.resize(Num_messages);