- made D_WriteUserInfoStrings memory safe.

Its callers are anything but for now but this function was the main blocker for refactoring so it had to come first.

src/d_net.cpp

@@ -1505,7 +1505,9 @@ bool DoArbitrate (void *userdata)
 		netbuffer[1] = consoleplayer;
 		netbuffer[9] = data->gotsetup[0];
 		stream = &netbuffer[10];
-		D_WriteUserInfoStrings (consoleplayer, &stream, true);
+		auto str = D_GetUserInfoStrings (consoleplayer, true);
+		memcpy(stream, str.GetChars(), str.Len() + 1);
+		stream += str.Len();
 		SendSetup (data->playersdetected, data->gotsetup, int(stream - netbuffer));
 	}
 	else
@@ -1520,7 +1522,9 @@ bool DoArbitrate (void *userdata)
 				{
 					netbuffer[1] = j;
 					stream = &netbuffer[9];
-					D_WriteUserInfoStrings (j, &stream, true);
+					auto str = D_GetUserInfoStrings(j, true);
+					memcpy(stream, str.GetChars(), str.Len() + 1);
+					stream += str.Len();
 					HSendPacket (i, int(stream - netbuffer));
 				}
 			}

src/d_netinf.h

@@ -58,7 +58,7 @@ bool D_SendServerInfoChange (FBaseCVar *cvar, UCVarValue value, ECVarType type);
 bool D_SendServerFlagChange (FBaseCVar *cvar, int bitnum, bool set, bool silent);
 void D_DoServerInfoChange (uint8_t **stream, bool singlebit);
 
-void D_WriteUserInfoStrings (int player, uint8_t **stream, bool compact=false);
+FString D_GetUserInfoStrings(int pnum, bool compact = false);
 void D_ReadUserInfoStrings (int player, uint8_t **stream, bool update);
 
 struct FPlayerColorSet;

src/d_netinfo.cpp

@@ -738,67 +738,65 @@ static int namesortfunc(const void *a, const void *b)
 	return stricmp(name1->GetChars(), name2->GetChars());
 }
 
-void D_WriteUserInfoStrings (int pnum, uint8_t **stream, bool compact)
+FString D_GetUserInfoStrings(int pnum, bool compact)
 {
-	if (pnum >= MAXPLAYERS)
+	FString result;
+	if (pnum >= 0 && pnum < MAXPLAYERS)
 	{
-		WriteByte (0, stream);
-		return;
-	}
+		userinfo_t* info = &players[pnum].userinfo;
+		TArray<TMap<FName, FBaseCVar*>::Pair*> userinfo_pairs(info->CountUsed());
+		TMap<FName, FBaseCVar*>::Iterator it(*info);
+		TMap<FName, FBaseCVar*>::Pair* pair;
+		UCVarValue cval;
 
-	userinfo_t *info = &players[pnum].userinfo;
-	TArray<TMap<FName, FBaseCVar *>::Pair *> userinfo_pairs(info->CountUsed());
-	TMap<FName, FBaseCVar *>::Iterator it(*info);
-	TMap<FName, FBaseCVar *>::Pair *pair;
-	UCVarValue cval;
-
-	// Create a simple array of all userinfo cvars
-	while (it.NextPair(pair))
-	{
-		userinfo_pairs.Push(pair);
-	}
-	// For compact mode, these need to be sorted. Verbose mode doesn't matter.
-	if (compact)
-	{
-		qsort(&userinfo_pairs[0], userinfo_pairs.Size(), sizeof(pair), userinfosortfunc);
-		// Compact mode is signified by starting the string with two backslash characters.
-		// We output one now. The second will be output as part of the first value.
-		*(*stream)++ = '\\';
-	}
-	for (unsigned int i = 0; i < userinfo_pairs.Size(); ++i)
-	{
-		pair = userinfo_pairs[i];
-
-		if (!compact)
-		{ // In verbose mode, prepend the cvar's name
-			*stream += sprintf(*((char **)stream), "\\%s", pair->Key.GetChars());
-		}
-		// A few of these need special handling for compatibility reasons.
-		switch (pair->Key.GetIndex())
+		// Create a simple array of all userinfo cvars
+		while (it.NextPair(pair))
 		{
-		case NAME_Gender:
-			*stream += sprintf(*((char **)stream), "\\%s",
-				*static_cast<FIntCVar *>(pair->Value) == GENDER_FEMALE ? "female" :
-				*static_cast<FIntCVar *>(pair->Value) == GENDER_NEUTER ? "neutral" :
-				*static_cast<FIntCVar *>(pair->Value) == GENDER_OBJECT ? "other" : "male");
-			break;
+			userinfo_pairs.Push(pair);
+		}
+		// For compact mode, these need to be sorted. Verbose mode doesn't matter.
+		if (compact)
+		{
+			qsort(&userinfo_pairs[0], userinfo_pairs.Size(), sizeof(pair), userinfosortfunc);
+			// Compact mode is signified by starting the string with two backslash characters.
+			// We output one now. The second will be output as part of the first value.
+			result += '\\';
+		}
+		for (unsigned int i = 0; i < userinfo_pairs.Size(); ++i)
+		{
+			pair = userinfo_pairs[i];
 
-		case NAME_PlayerClass:
-			*stream += sprintf(*((char **)stream), "\\%s", info->GetPlayerClassNum() == -1 ? "Random" :
-				D_EscapeUserInfo(info->GetPlayerClassType()->GetDisplayName().GetChars()).GetChars());
-			break;
+			if (!compact)
+			{ // In verbose mode, prepend the cvar's name
+				result.AppendFormat("\\%s", pair->Key.GetChars());
+			}
+			// A few of these need special handling for compatibility reasons.
+			switch (pair->Key.GetIndex())
+			{
+			case NAME_Gender:
+				result.AppendFormat("\\%s",
+					*static_cast<FIntCVar*>(pair->Value) == GENDER_FEMALE ? "female" :
+					*static_cast<FIntCVar*>(pair->Value) == GENDER_NEUTER ? "neutral" :
+					*static_cast<FIntCVar*>(pair->Value) == GENDER_OBJECT ? "other" : "male");
+				break;
 
-		case NAME_Skin:
-			*stream += sprintf(*((char **)stream), "\\%s", D_EscapeUserInfo(Skins[info->GetSkin()].Name).GetChars());
-			break;
+			case NAME_PlayerClass:
+				result.AppendFormat("\\%s", info->GetPlayerClassNum() == -1 ? "Random" :
+					D_EscapeUserInfo(info->GetPlayerClassType()->GetDisplayName().GetChars()).GetChars());
+				break;
 
-		default:
-			cval = pair->Value->GetGenericRep(CVAR_String);
-			*stream += sprintf(*((char **)stream), "\\%s", cval.String);
-			break;
+			case NAME_Skin:
+				result.AppendFormat("\\%s", D_EscapeUserInfo(Skins[info->GetSkin()].Name).GetChars());
+				break;
+
+			default:
+				cval = pair->Value->GetGenericRep(CVAR_String);
+				result.AppendFormat("\\%s", cval.String);
+				break;
+			}
 		}
 	}
-	*(*stream)++ = '\0';
+	return result;
 }
 
 void D_ReadUserInfoStrings (int pnum, uint8_t **stream, bool update)

src/g_game.cpp

@@ -2618,10 +2618,12 @@ void G_BeginRecording (const char *startmap)
 	{
 		if (playeringame[i])
 		{
-			StartChunk (UINF_ID, &demo_p);
-			WriteByte ((uint8_t)i, &demo_p);
-			D_WriteUserInfoStrings (i, &demo_p);
-			FinishChunk (&demo_p);
+			StartChunk(UINF_ID, &demo_p);
+			WriteByte((uint8_t)i, &demo_p);
+			auto str = D_GetUserInfoStrings(i);
+			memcpy(demo_p, str.GetChars(), str.Len() + 1);
+			demo_p += str.Len();
+			FinishChunk(&demo_p);
 		}
 	}