fix multi crash loading campaigns (#6871)

A minor bug in #6077 resulted in a crash loading campaigns on the multi ui.
The bug itself was not fatal, however it triggered a cascade of poor error
handling which led to a crash. A lot of these issues date back to retail.
This fixes the minor bug, and hopefully adds enough error handling to
preemptively squash a repeat of the crash problem.

Author: notimaginative

code/mission/missioncampaign.cpp

@@ -100,14 +100,30 @@ campaign Campaign;
  * In the type field, we return if the campaign is a single player or multiplayer campaign.  
  * The type field will only be valid if the name returned is non-NULL
  */
-int mission_campaign_get_info(const char *filename, char *name, int *type, int *max_players, char **desc, char **first_mission)
+bool mission_campaign_get_info(const char *filename, SCP_string &name, int *type, int *max_players, char **desc, char **first_mission)
 {
-	int i, success = 0;
-	char campaign_type[NAME_LENGTH], fname[MAX_FILENAME_LEN];
+	int i, success = false;
+	SCP_string campaign_type;
+	char fname[MAX_FILENAME_LEN];
 
-	Assert( name != NULL );
 	Assert( type != NULL );
 
+	// make sure outputs always have sane values
+	name.clear();
+	*type = -1;
+
+	if (max_players) {
+		*max_players = 0;
+	}
+
+	if (desc) {
+		*desc = nullptr;
+	}
+
+	if (first_mission) {
+		*first_mission = nullptr;
+	}
+
 	strncpy(fname, filename, MAX_FILENAME_LEN - 1);
 	auto fname_len = strlen(fname);
 	if ((fname_len < 4) || stricmp(fname + fname_len - 4, FS_CAMPAIGN_FILE_EXT) != 0){
@@ -116,31 +132,30 @@ int mission_campaign_get_info(const char *filename, char *name, int *type, int *
 	}
 	Assert(fname_len < MAX_FILENAME_LEN);
 
-	*type = -1;
 	do {
 		try
 		{
 			read_file_text(fname);
 			reset_parse();
 
 			required_string("$Name:");
-			stuff_string(name, F_NAME, NAME_LENGTH);
-			if (name == NULL) {
+			stuff_string(name, F_NAME);
+			if (name.empty()) {
 				nprintf(("Warning", "No name found for campaign file %s\n", filename));
 				break;
 			}
 
 			required_string("$Type:");
-			stuff_string(campaign_type, F_NAME, NAME_LENGTH);
+			stuff_string(campaign_type, F_NAME);
 
 			for (i = 0; i < MAX_CAMPAIGN_TYPES; i++) {
-				if (!stricmp(campaign_type, campaign_types[i])) {
+				if (!stricmp(campaign_type.c_str(), campaign_types[i])) {
 					*type = i;
 				}
 			}
 
-			if (name == NULL) {
-				Warning(LOCATION, "Invalid campaign type \"%s\"\n", campaign_type);
+			if (*type < 0) {
+				Warning(LOCATION, "Invalid campaign type \"%s\"\n", campaign_type.c_str());
 				break;
 			}
 
@@ -166,7 +181,7 @@ int mission_campaign_get_info(const char *filename, char *name, int *type, int *
 
 			// if we found a valid campaign type
 			if ((*type) >= 0) {
-				success = 1;
+				success = true;
 			}
 		}
 		catch (const parse::ParseException& e)
@@ -176,7 +191,6 @@ int mission_campaign_get_info(const char *filename, char *name, int *type, int *
 		}
 	} while (0);
 
-	Assert(success);
 	return success;
 }
 
@@ -250,7 +264,7 @@ void mission_campaign_free_list()
 
 int mission_campaign_maybe_add(const char *filename)
 {
-	char name[NAME_LENGTH];
+	SCP_string name;
 	char *desc = NULL;
 	int type, max_players;
 
@@ -261,7 +275,7 @@ int mission_campaign_maybe_add(const char *filename)
 
 	if ( mission_campaign_get_info( filename, name, &type, &max_players, &desc) ) {
 		if ( !MC_multiplayer && (type == CAMPAIGN_TYPE_SINGLE) ) {
-			Campaign_names[Num_campaigns] = vm_strdup(name);
+			Campaign_names[Num_campaigns] = vm_strdup(name.c_str());
 
 			if (MC_desc)
 				Campaign_descs[Num_campaigns] = desc;

code/mission/missioncampaign.h

@@ -213,7 +213,7 @@ extern void mission_campaign_save_persistent( int type, int index );
 // execute the corresponding mission_campaign_savefile functions.
 
 // get name and type of specified campaign file
-int mission_campaign_get_info(const char *filename, char *name, int *type, int *max_players, char **desc = nullptr, char **first_mission = nullptr);
+bool mission_campaign_get_info(const char *filename, SCP_string &name, int *type, int *max_players, char **desc = nullptr, char **first_mission = nullptr);
 
 // get a listing of missions in a campaign
 int mission_campaign_get_mission_list(const char *filename, char **list, int max);

code/mission/missionparse.cpp

@@ -6675,7 +6675,11 @@ int get_mission_info(const char *filename, mission *mission_p, bool basic, bool
 {
 	static SCP_string real_fname_buf;
 	const char *real_fname = nullptr;
-	
+
+	if ( !filename || !strlen(filename) ) {
+		return -1;
+	}
+
 	if (filename_is_full_path) {
 		real_fname = filename;
 	} else {

code/network/multi_options.cpp

@@ -750,7 +750,7 @@ void multi_options_process_packet(unsigned char *data, header *hinfo)
 	// get mission choice options
 	case MULTI_OPTION_MISSION: {
 		netgame_info ng;
-		char title[NAME_LENGTH+1];
+		SCP_string title;
 		int campaign_type,max_players;
 
 		Assert(Game_mode & GM_STANDALONE_SERVER);
@@ -779,7 +779,6 @@ void multi_options_process_packet(unsigned char *data, header *hinfo)
 
 			// set the netgame max players here if the filename has changed
 			if(strcmp(Netgame.campaign_name,ng.campaign_name) != 0){				
-				memset(title,0,NAME_LENGTH+1);			
 				if(!mission_campaign_get_info(ng.campaign_name,title,&campaign_type,&max_players)){
 					Netgame.max_players = 0;
 				} else {

code/network/multiui.cpp

@@ -4535,7 +4535,7 @@ void multi_create_list_load_campaigns()
 {
 	int idx, file_count;
 	int campaign_type,max_players;
-	char title[255];
+	SCP_string title;
 	char wild_card[10];
 	char **file_list = NULL;
 
@@ -4717,13 +4717,13 @@ void multi_create_list_do()
 // so we can set the data without bothering to check the UI anymore
 void multi_create_list_set_item(int abs_index, int mode) {
 
-	int campaign_type, max_players;
-	char title[NAME_LENGTH + 1];
+	int campaign_type = -1, max_players = 0;
+	SCP_string title;
 	netgame_info ng_temp;
 	netgame_info* ng;
 	multi_create_info* mcip = NULL;
 
-	char* campaign_desc;
+	char* campaign_desc = nullptr;
 
 	// if not on the standalone server
 	if (Net_player->flags & NETINFO_FLAG_AM_MASTER) {
@@ -4738,7 +4738,7 @@ void multi_create_list_set_item(int abs_index, int mode) {
 	if (mode == MULTI_CREATE_SHOW_MISSIONS) {
 		strcpy(ng->mission_name, Multi_create_mission_list[abs_index].filename);
 	} else {
-		strcpy(ng->mission_name, Multi_create_campaign_list[abs_index].filename);
+		strcpy(ng->campaign_name, Multi_create_campaign_list[abs_index].filename);
 	}
 
 	// make sure the netgame type is properly set
@@ -4819,24 +4819,22 @@ void multi_create_list_set_item(int abs_index, int mode) {
 
 		// if not on the standalone server
 		if (Net_player->flags & NETINFO_FLAG_AM_MASTER) {
-			memset(title, 0, sizeof(title));
 			// get the campaign info
 			if (!mission_campaign_get_info(ng->campaign_name,
 					title,
 					&campaign_type,
 					&max_players,
 					&campaign_desc,
-					&first_mission)) {
+					&first_mission))
+			{
+				nprintf(("Network", "MC: Failed to get campaign info for '%s'!\n", ng->campaign_name));
 				memset(ng->campaign_name, 0, sizeof(ng->campaign_name));
-				ng->max_players = 0;
-			}
-			// if we successfully got the info
-			else {
-				memset(ng->title, 0, NAME_LENGTH + 1);
-				strcpy_s(ng->title, title);
-				ng->max_players = max_players;
 			}
 
+			memset(ng->title, 0, sizeof(ng->title));
+			strcpy_s(ng->title, title.c_str());
+			ng->max_players = max_players;
+
 			nprintf(("Network", "MC MAX PLAYERS : %d\n", ng->max_players));
 
 			// set the information area text

code/playerman/managepilot.cpp

@@ -170,26 +170,17 @@ int local_num_campaigns = 0;
 
 int campaign_file_list_filter(const char *filename)
 {
-	char name[NAME_LENGTH];
-	char *desc = NULL;
+	SCP_string name;
 	int type, max_players;
 
-	if ( mission_campaign_get_info( filename, name, &type, &max_players, &desc) ) {
+	if ( mission_campaign_get_info( filename, name, &type, &max_players) ) {
 		if ( type == CAMPAIGN_TYPE_SINGLE) {
-			Campaign_names[local_num_campaigns] = vm_strdup(name);
+			Campaign_names[local_num_campaigns] = vm_strdup(name.c_str());
 			local_num_campaigns++;
-
-			// here we *do* free the campaign description because we are not saving the pointer.
-			if (desc != NULL)
-				vm_free(desc);
-
 			return 1;
 		}
 	}
 
-	if (desc != NULL)
-		vm_free(desc);
- 
 	return 0;
 }
 

code/scripting/api/libs/mission.cpp

@@ -1847,7 +1847,9 @@ ADE_FUNC(loadMission, l_Mission, "string missionName", "Loads a mission", "boole
 	gr_post_process_set_defaults();
 
 	//NOW do the loading stuff
-	get_mission_info(s, &The_mission, false);
+	if (get_mission_info(s, &The_mission, false))
+		return ADE_RETURN_FALSE;
+
 	game_level_init();
 
 	if(!mission_load(s))