Merge #6313

6313: Use `str_copy` instead of `str_format`, minor fixes and refactoring r=Chairn a=Robyt3



## Checklist

- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>

src/base/system.cpp

@@ -1117,14 +1117,14 @@ void net_addr_str_v6(const unsigned short ip[8], int port, char *buffer, int buf
 		longest_seq_len = 0;
 		longest_seq_start = -1;
 	}
-	w += str_format(buffer + w, buffer_size - w, "[");
+	w += str_copy(buffer + w, "[", buffer_size - w);
 	for(i = 0; i < 8; i++)
 	{
 		if(longest_seq_start <= i && i < longest_seq_start + longest_seq_len)
 		{
 			if(i == longest_seq_start)
 			{
-				w += str_format(buffer + w, buffer_size - w, "::");
+				w += str_copy(buffer + w, "::", buffer_size - w);
 			}
 		}
 		else
@@ -1133,7 +1133,7 @@ void net_addr_str_v6(const unsigned short ip[8], int port, char *buffer, int buf
 			w += str_format(buffer + w, buffer_size - w, "%s%x", colon, ip[i]);
 		}
 	}
-	w += str_format(buffer + w, buffer_size - w, "]");
+	w += str_copy(buffer + w, "]", buffer_size - w);
 	if(port >= 0)
 	{
 		str_format(buffer + w, buffer_size - w, ":%d", port);
@@ -2633,11 +2633,11 @@ void str_append(char *dst, const char *src, int dst_size)
 	str_utf8_fix_truncation(dst);
 }
 
-void str_copy(char *dst, const char *src, int dst_size)
+int str_copy(char *dst, const char *src, int dst_size)
 {
 	dst[0] = '\0';
 	strncat(dst, src, dst_size - 1);
-	str_utf8_fix_truncation(dst);
+	return str_utf8_fix_truncation(dst);
 }
 
 void str_utf8_truncate(char *dst, int dst_size, const char *src, int truncation_len)

src/base/system.h

@@ -1192,10 +1192,12 @@ void str_append(char *dst, const char *src, int dst_size);
  * @param src String to be copied.
  * @param dst_size Size of the buffer dst.
  *
+ * @return Length of written string, even if it has been truncated
+ *
  * @remark The strings are treated as zero-terminated strings.
  * @remark Guarantees that dst string will contain zero-termination.
  */
-void str_copy(char *dst, const char *src, int dst_size);
+int str_copy(char *dst, const char *src, int dst_size);
 
 /**
  * Truncates a utf8 encoded string to a given length.

src/engine/client/client.cpp

@@ -1996,9 +1996,7 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket, int Conn, bool Dummy)
 							// to compress this snapshot. force the server to resync
 							if(g_Config.m_Debug)
 							{
-								char aBuf[256];
-								str_format(aBuf, sizeof(aBuf), "error, couldn't find the delta snapshot");
-								m_pConsole->Print(IConsole::OUTPUT_LEVEL_DEBUG, "client", aBuf);
+								m_pConsole->Print(IConsole::OUTPUT_LEVEL_DEBUG, "client", "error, couldn't find the delta snapshot");
 							}
 
 							// ack snapshot

src/engine/server/databases/connection_pool.cpp

@@ -432,12 +432,12 @@ void CWorker::Print(IConsole *pConsole, CDbConnectionPool::Mode DatabaseMode)
 /* static */
 bool CDbConnectionPool::ExecSqlFunc(IDbConnection *pConnection, CSqlExecData *pData, Write w)
 {
-	char aError[256] = "error message not initialized";
 	if(pConnection == nullptr)
 	{
-		str_format(aError, sizeof(aError), "No database given");
+		dbg_msg("sql", "No database given");
 		return false;
 	}
+	char aError[256] = "unknown error";
 	if(pConnection->Connect(aError, sizeof(aError)))
 	{
 		dbg_msg("sql", "failed connecting to db: %s", aError);

src/game/client/components/statboard.cpp

@@ -316,7 +316,7 @@ void CStatboard::RenderGlobalStats()
 		// RATIO
 		{
 			if(pStats->m_Deaths == 0)
-				str_format(aBuf, sizeof(aBuf), "--");
+				str_copy(aBuf, "--");
 			else
 				str_format(aBuf, sizeof(aBuf), "%.2f", (float)(pStats->m_Frags) / pStats->m_Deaths);
 			tw = TextRender()->TextWidth(0, FontSize, aBuf, -1, -1.0f);
@@ -469,7 +469,7 @@ void CStatboard::FormatStats(char *pDest, size_t DestSize)
 	}
 
 	char aPlayerStats[1024 * VANILLA_MAX_CLIENTS];
-	str_format(aPlayerStats, sizeof(aPlayerStats), "Local-player,Team,Name,Clan,Score,Frags,Deaths,Suicides,F/D-ratio,Net,FPM,Spree,Best,Hammer-F/D,Gun-F/D,Shotgun-F/D,Grenade-F/D,Laser-F/D,Ninja-F/D,GameWithFlags,Flag-grabs,Flag-captures\n");
+	str_copy(aPlayerStats, "Local-player,Team,Name,Clan,Score,Frags,Deaths,Suicides,F/D-ratio,Net,FPM,Spree,Best,Hammer-F/D,Gun-F/D,Shotgun-F/D,Grenade-F/D,Laser-F/D,Ninja-F/D,GameWithFlags,Flag-grabs,Flag-captures\n");
 	for(int i = 0; i < NumPlayers; i++)
 	{
 		const CNetObj_PlayerInfo *pInfo = apPlayers[i];

src/game/collision.cpp

@@ -1032,31 +1032,31 @@ int CCollision::Entity(int x, int y, int Layer) const
 {
 	if((0 > x || x >= m_Width) || (0 > y || y >= m_Height))
 	{
-		char aBuf[12];
+		const char *pName;
 		switch(Layer)
 		{
 		case LAYER_GAME:
-			str_format(aBuf, sizeof(aBuf), "Game");
+			pName = "Game";
 			break;
 		case LAYER_FRONT:
-			str_format(aBuf, sizeof(aBuf), "Front");
+			pName = "Front";
 			break;
 		case LAYER_SWITCH:
-			str_format(aBuf, sizeof(aBuf), "Switch");
+			pName = "Switch";
 			break;
 		case LAYER_TELE:
-			str_format(aBuf, sizeof(aBuf), "Tele");
+			pName = "Tele";
 			break;
 		case LAYER_SPEEDUP:
-			str_format(aBuf, sizeof(aBuf), "Speedup");
+			pName = "Speedup";
 			break;
 		case LAYER_TUNE:
-			str_format(aBuf, sizeof(aBuf), "Tune");
+			pName = "Tune";
 			break;
 		default:
-			str_format(aBuf, sizeof(aBuf), "Unknown");
+			pName = "Unknown";
 		}
-		dbg_msg("collision", "something is VERY wrong with the %s layer please report this at https://github.com/ddnet/ddnet, you will need to post the map as well and any steps that u think may have led to this", aBuf);
+		dbg_msg("collision", "something is VERY wrong with the %s layer please report this at https://github.com/ddnet/ddnet, you will need to post the map as well and any steps that u think may have led to this", pName);
 		return 0;
 	}
 	switch(Layer)

src/game/editor/editor.cpp

@@ -5159,8 +5159,8 @@ void CEditor::RenderEnvelopeEditor(CUIRect View)
 			s_pID = nullptr;
 
 			// update displayed text
-			str_format(s_aStrCurTime, sizeof(s_aStrCurTime), "0.000");
-			str_format(s_aStrCurValue, sizeof(s_aStrCurValue), "0.000");
+			str_copy(s_aStrCurTime, "0.000");
+			str_copy(s_aStrCurValue, "0.000");
 		}
 
 		{
@@ -5253,8 +5253,8 @@ void CEditor::RenderEnvelopeEditor(CUIRect View)
 								s_pID = nullptr;
 
 								// update displayed text
-								str_format(s_aStrCurTime, sizeof(s_aStrCurTime), "0.000");
-								str_format(s_aStrCurValue, sizeof(s_aStrCurValue), "0.000");
+								str_copy(s_aStrCurTime, "0.000");
+								str_copy(s_aStrCurValue, "0.000");
 							}
 
 							pEnvelope->m_vPoints.erase(pEnvelope->m_vPoints.begin() + i);

src/game/server/ddracechat.cpp

@@ -1445,7 +1445,7 @@ void CGameContext::ConSetTimerType(IConsole::IResult *pResult, void *pUserData)
 	if(pPlayer->m_TimerType <= CPlayer::TIMERTYPE_SIXUP && pPlayer->m_TimerType >= CPlayer::TIMERTYPE_GAMETIMER)
 		str_format(aBuf, sizeof(aBuf), "Timer is displayed in %s", s_aaMsg[pPlayer->m_TimerType]);
 	else if(pPlayer->m_TimerType == CPlayer::TIMERTYPE_NONE)
-		str_format(aBuf, sizeof(aBuf), "Timer isn't displayed.");
+		str_copy(aBuf, "Timer isn't displayed.");
 
 	pSelf->Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "chatresp", aBuf);
 }

src/game/server/ddracecommands.cpp

@@ -700,15 +700,11 @@ void CGameContext::ConModerate(IConsole::IResult *pResult, void *pUserData)
 	CPlayer *pPlayer = pSelf->m_apPlayers[pResult->m_ClientID];
 	pPlayer->m_Moderating = !pPlayer->m_Moderating;
 
-	char aBuf[256];
-
 	if(!HadModerator && pPlayer->m_Moderating)
-		str_format(aBuf, sizeof(aBuf), "Server kick/spec votes will now be actively moderated.");
+		pSelf->SendChat(-1, CHAT_ALL, "Server kick/spec votes will now be actively moderated.", 0);
 
 	if(!pSelf->PlayerModerating())
-		str_format(aBuf, sizeof(aBuf), "Server kick/spec votes are no longer actively moderated.");
-
-	pSelf->SendChat(-1, CHAT_ALL, aBuf, 0);
+		pSelf->SendChat(-1, CHAT_ALL, "Server kick/spec votes are no longer actively moderated.", 0);
 
 	if(pPlayer->m_Moderating)
 		pSelf->SendChatTarget(pResult->m_ClientID, "Active moderator mode enabled for you.");

src/game/server/entities/character.cpp

@@ -1733,7 +1733,7 @@ void CCharacter::HandleTiles(int Index)
 		{
 			char aBuf[256];
 			if(NewJumps == -1)
-				str_format(aBuf, sizeof(aBuf), "You only have your ground jump now");
+				str_copy(aBuf, "You only have your ground jump now");
 			else if(NewJumps == 1)
 				str_format(aBuf, sizeof(aBuf), "You can jump %d time", NewJumps);
 			else

src/game/server/gamecontext.cpp

@@ -1021,18 +1021,14 @@ void CGameContext::OnTick()
 			}
 			else if(m_VoteEnforce == VOTE_ENFORCE_YES_ADMIN)
 			{
-				char aBuf[64];
-				str_format(aBuf, sizeof(aBuf), "Vote passed enforced by authorized player");
 				Console()->ExecuteLine(m_aVoteCommand, m_VoteEnforcer);
-				SendChat(-1, CGameContext::CHAT_ALL, aBuf, -1, CHAT_SIX);
+				SendChat(-1, CGameContext::CHAT_ALL, "Vote passed enforced by authorized player", -1, CHAT_SIX);
 				EndVote();
 			}
 			else if(m_VoteEnforce == VOTE_ENFORCE_NO_ADMIN)
 			{
-				char aBuf[64];
-				str_format(aBuf, sizeof(aBuf), "Vote failed enforced by authorized player");
 				EndVote();
-				SendChat(-1, CGameContext::CHAT_ALL, aBuf, -1, CHAT_SIX);
+				SendChat(-1, CGameContext::CHAT_ALL, "Vote failed enforced by authorized player", -1, CHAT_SIX);
 			}
 			//else if(m_VoteEnforce == VOTE_ENFORCE_NO || time_get() > m_VoteCloseTime)
 			else if(m_VoteEnforce == VOTE_ENFORCE_NO || (time_get() > m_VoteCloseTime && g_Config.m_SvVoteMajority))
@@ -4017,24 +4013,21 @@ void CGameContext::Whisper(int ClientID, char *pStr)
 	*pStr = 0;
 	pStr++;
 
-	char *pMessage = pStr;
-	char aBuf[256];
-
 	if(Error)
 	{
-		str_format(aBuf, sizeof(aBuf), "Invalid whisper");
-		SendChatTarget(ClientID, aBuf);
+		SendChatTarget(ClientID, "Invalid whisper");
 		return;
 	}
 
 	if(Victim >= MAX_CLIENTS || !CheckClientID2(Victim))
 	{
+		char aBuf[256];
 		str_format(aBuf, sizeof(aBuf), "No player with name \"%s\" found", pName);
 		SendChatTarget(ClientID, aBuf);
 		return;
 	}
 
-	WhisperID(ClientID, Victim, pMessage);
+	WhisperID(ClientID, Victim, pStr);
 }
 
 void CGameContext::WhisperID(int ClientID, int VictimID, const char *pMessage)
@@ -4141,7 +4134,7 @@ void CGameContext::List(int ClientID, const char *pFilter)
 	if(pFilter[0])
 		str_format(aBuf, sizeof(aBuf), "Listing players with \"%s\" in name:", pFilter);
 	else
-		str_format(aBuf, sizeof(aBuf), "Listing all players:");
+		str_copy(aBuf, "Listing all players:");
 	SendChatTarget(ClientID, aBuf);
 	for(int i = 0; i < MAX_CLIENTS; i++)
 	{

src/game/server/teehistorian.cpp

@@ -161,7 +161,7 @@ void CTeeHistorian::WriteHeader(const CGameInfo *pGameInfo)
 #undef MACRO_CONFIG_COL
 #undef MACRO_CONFIG_STR
 
-	str_format(aJson, sizeof(aJson), "},\"tuning\":{");
+	str_copy(aJson, "},\"tuning\":{");
 	Write(aJson, str_length(aJson));
 
 	First = true;
@@ -180,7 +180,7 @@ void CTeeHistorian::WriteHeader(const CGameInfo *pGameInfo)
 #include <game/tuning.h>
 #undef MACRO_TUNING_PARAM
 
-	str_format(aJson, sizeof(aJson), "},\"uuids\":[");
+	str_copy(aJson, "},\"uuids\":[");
 	Write(aJson, str_length(aJson));
 
 	for(int i = 0; i < pGameInfo->m_pUuids->NumUuids(); i++)
@@ -191,7 +191,7 @@ void CTeeHistorian::WriteHeader(const CGameInfo *pGameInfo)
 		Write(aJson, str_length(aJson));
 	}
 
-	str_format(aJson, sizeof(aJson), "]}");
+	str_copy(aJson, "]}");
 	Write(aJson, str_length(aJson));
 	Write("", 1); // Null termination.
 }