From: Cloudwalk <cloudwalk009@gmail.com>
Date: Mon, 3 Aug 2020 19:26:06 +0000 (-0400)
Subject: (WIP) cmd: Reimplement console buffer system as a linked list
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=a3226e770b06cd73895a0ff68146ff8b244976a9;p=xonotic%2Fdarkplaces.git

(WIP) cmd: Reimplement console buffer system as a linked list

This has several advantages, namely, multiple command interpreters
may share the same console buffer on the same thread, and each
command can be executed in order, eliminating any potential bugs
related to that.

Things left to do:

* Merge deferred commands into the same system and reenable
  the defer command.
* Use a cyclic doubly linked list instead of the garbage I
  wrote.
---

diff --git a/cl_demo.c b/cl_demo.c
index b8445419..5acbaa4e 100644
--- a/cl_demo.c
+++ b/cl_demo.c
@@ -275,7 +275,7 @@ void CL_ReadDemoMessage(void)
 			CL_ParseServerMessage();
 
 			if (cls.signon != SIGNONS)
-				Cbuf_Execute(&cmd_client); // immediately execute svc_stufftext if in the demo before connect!
+				Cbuf_Execute((&cmd_client)->cbuf); // immediately execute svc_stufftext if in the demo before connect!
 
 			// In case the demo contains a "svc_disconnect" message
 			if (!cls.demoplayback)
diff --git a/cmd.c b/cmd.c
index 65c81258..70b3e83f 100644
--- a/cmd.c
+++ b/cmd.c
@@ -47,14 +47,14 @@ qboolean host_stuffcmdsrun = false;
 
 //=============================================================================
 
-void Cbuf_Lock(cmd_state_t *cmd)
+void Cbuf_Lock(cbuf_t *cbuf)
 {
-	Thread_LockMutex(cmd->text_mutex);
+	Thread_LockMutex(cbuf->lock);
 }
 
-void Cbuf_Unlock(cmd_state_t *cmd)
+void Cbuf_Unlock(cbuf_t *cbuf)
 {
-	Thread_UnlockMutex(cmd->text_mutex);
+	Thread_UnlockMutex(cbuf->lock);
 }
 
 
@@ -69,7 +69,7 @@ bind g "impulse 5 ; +attack ; wait ; -attack ; impulse 2"
 */
 static void Cmd_Wait_f (cmd_state_t *cmd)
 {
-	cmd->wait = true;
+	cmd->cbuf->wait = true;
 }
 
 /*
@@ -180,6 +180,106 @@ static void Cmd_Centerprint_f (cmd_state_t *cmd)
 =============================================================================
 */
 
+/*
+============
+Cbuf_ParseText
+
+Parses Quake console command-line
+Allocates a cbuf_cmd_t for each individual
+command. Returns a pointer to the last element.
+============
+*/
+static cbuf_cmd_t *Cbuf_ParseText(cmd_state_t *cmd, cbuf_t *cbuf, cbuf_cmd_t **start, const char *text)
+{
+	int i = 0;
+	cbuf_cmd_t *current = NULL;
+	cbuf_cmd_t *end = NULL;
+	qboolean quotes = false;
+	qboolean comment = false;
+	qboolean escaped = false;
+	qboolean mark = false;
+	char *offset = NULL;
+	size_t cmdsize = 0;
+
+	// It has to be NULL because it writes into it first
+	if(*start)
+		Sys_Error("Cbuf_ParseText: 'start' is non-NULL. This should never happen. Please report this to a developer.\n");
+
+	while(text[i])
+	{
+		switch (text[i])
+		{
+			case '"':
+				if (!escaped)
+					quotes = !quotes;
+				break;
+			case '\\':
+				if (!escaped && quotes)
+					escaped = true;
+				break;
+			case '/':
+				if (!quotes && !comment && text[i+1] == '/' && (i == 0 || ISWHITESPACE(text[i-1])))
+					comment = true;
+				break;
+			case ';':
+				if(!comment && !quotes)
+					mark = true;
+				break;
+			case '\r':
+			case '\n':
+				mark = true;
+				break;
+			default:
+			// If there's no trailing newline, mark for allocation
+				if(text[i+1] == 0)
+				{
+					mark = true;
+					cmdsize++;
+				}
+				break;
+		}
+
+		if(!offset && !comment && !mark)
+			offset = (char *)&text[i];
+
+		if(!comment && !mark && offset)
+			cmdsize++;
+
+		if(mark)
+		{
+			comment = false;
+			quotes = false;
+			escaped = false;
+
+			if(offset)
+			{
+				current = (cbuf_cmd_t *)Z_Malloc(sizeof(cbuf_cmd_t));
+				if(end)
+					end->next = current;
+				if(!*start)
+					*start = current;
+				current->prev = end;
+
+				current->text = (char *)Z_Malloc(cmdsize + 1);
+				memcpy(current->text, offset, cmdsize);
+				current->source = cmd;
+
+				end = current;
+				end->next = NULL;
+				current = end->next;
+				offset = NULL;
+				cbuf->size += cmdsize;
+			}
+
+			cmdsize = 0;
+			mark = false;
+		}
+		i++;
+	}
+
+	return end;
+}
+
 /*
 ============
 Cbuf_AddText
@@ -189,16 +289,33 @@ Adds command text at the end of the buffer
 */
 void Cbuf_AddText (cmd_state_t *cmd, const char *text)
 {
-	int		l;
+	size_t l = strlen(text);
+	cbuf_t *cbuf = cmd->cbuf;
+	cbuf_cmd_t *add = NULL, *newend = NULL;
 
-	l = (int)strlen(text);
+	Cbuf_Lock(cbuf);
 
-	Cbuf_Lock(cmd);
-	if (cmd->text.maxsize - cmd->text.cursize <= l)
+	if (cbuf->maxsize - cbuf->size <= l)
 		Con_Print("Cbuf_AddText: overflow\n");
 	else
-		SZ_Write(&cmd->text, (const unsigned char *)text, l);
-	Cbuf_Unlock(cmd);
+	{
+		newend = Cbuf_ParseText(cmd, cbuf, &add, text);
+		if(!newend)
+		{
+			Con_Printf("Cbuf_InsertText: newend = NULL\n");
+			return;
+		}
+
+		if(!cbuf->start)
+			cbuf->start = add;
+		if(cbuf->end)
+		{
+			cbuf->end->next = add;
+			add->prev = cbuf->end;
+		}
+		cbuf->end = newend;	
+	}
+	Cbuf_Unlock(cbuf);
 }
 
 
@@ -213,19 +330,35 @@ FIXME: actually change the command buffer to do less copying
 */
 void Cbuf_InsertText (cmd_state_t *cmd, const char *text)
 {
+	cbuf_t *cbuf = cmd->cbuf;
+	cbuf_cmd_t *insert = NULL, *oldstart = NULL, *newend = NULL;
 	size_t l = strlen(text);
-	Cbuf_Lock(cmd);
+
+	Cbuf_Lock(cbuf);
+
 	// we need to memmove the existing text and stuff this in before it...
-	if (cmd->text.cursize + l >= (size_t)cmd->text.maxsize)
+	if (cmd->cbuf->size + l >= (size_t)cmd->cbuf->maxsize)
 		Con_Print("Cbuf_InsertText: overflow\n");
 	else
 	{
-		// we don't have a SZ_Prepend, so...
-		memmove(cmd->text.data + l, cmd->text.data, cmd->text.cursize);
-		cmd->text.cursize += (int)l;
-		memcpy(cmd->text.data, text, l);
+		newend = Cbuf_ParseText(cmd, cbuf, &insert, text);
+		if(!newend)
+		{
+			Con_Printf("Cbuf_InsertText: newend = NULL\n");
+			return;
+		}
+
+		oldstart = cbuf->start;
+		cbuf->start = insert;
+		
+		if(!cbuf->end)
+			cbuf->end = newend;
+		if(oldstart)
+			oldstart->prev = newend;
+		newend->next = oldstart;
 	}
-	Cbuf_Unlock(cmd);
+
+	Cbuf_Unlock(cbuf);
 }
 
 /*
@@ -275,80 +408,24 @@ Cbuf_Execute
 ============
 */
 static qboolean Cmd_PreprocessString(cmd_state_t *cmd, const char *intext, char *outtext, unsigned maxoutlen, cmdalias_t *alias );
-void Cbuf_Execute (cmd_state_t *cmd)
+void Cbuf_Execute (cbuf_t *cbuf)
 {
-	int i;
-	char *text;
-	char line[MAX_INPUTLINE];
+	cbuf_cmd_t *current;
 	char preprocessed[MAX_INPUTLINE];
 	char *firstchar;
-	qboolean quotes;
-	char *comment;
 
 	// LadyHavoc: making sure the tokenizebuffer doesn't get filled up by repeated crashes
-	cmd->tokenizebufferpos = 0;
+	//current->tokenizebufferpos = 0;
 
-	while (cmd->text.cursize)
+	while (cbuf->start)
 	{
-// find a \n or ; line break
-		text = (char *)cmd->text.data;
-
-		quotes = false;
-		comment = NULL;
-		for (i=0 ; i < cmd->text.cursize ; i++)
-		{
-			if(!comment)
-			{
-				if (text[i] == '"')
-					quotes = !quotes;
-
-				if(quotes)
-				{
-					// make sure i doesn't get > cursize which causes a negative
-					// size in memmove, which is fatal --blub
-					if (i < (cmd->text.cursize-1) && (text[i] == '\\' && (text[i+1] == '"' || text[i+1] == '\\')))
-						i++;
-				}
-				else
-				{
-					if(text[i] == '/' && text[i + 1] == '/' && (i == 0 || ISWHITESPACE(text[i-1])))
-						comment = &text[i];
-					if(text[i] == ';')
-						break;	// don't break if inside a quoted string or comment
-				}
-			}
 
-			if (text[i] == '\r' || text[i] == '\n')
-				break;
-		}
-
-		// better than CRASHING on overlong input lines that may SOMEHOW enter the buffer
-		if(i >= MAX_INPUTLINE)
-		{
-			Con_Printf(CON_WARN "Warning: console input buffer had an overlong line. Ignored.\n");
-			line[0] = 0;
-		}
-		else
-		{
-			memcpy (line, text, comment ? (comment - text) : i);
-			line[comment ? (comment - text) : i] = 0;
-		}
-
-// delete the text from the command buffer and move remaining commands down
-// this is necessary because commands (exec, alias) can insert data at the
-// beginning of the text buffer
-
-		if (i == cmd->text.cursize)
-			cmd->text.cursize = 0;
-		else
-		{
-			i++;
-			cmd->text.cursize -= i;
-			memmove (cmd->text.data, text+i, cmd->text.cursize);
-		}
-
-// execute the command line
-		firstchar = line;
+	// delete the text from the command buffer and move remaining commands down
+	// this is necessary because commands (exec, alias) can insert data at the
+	// beginning of the text buffer
+		current = cbuf->start;
+		cbuf->start = current->next;		
+		firstchar = current->text;
 		while(*firstchar && ISWHITESPACE(*firstchar))
 			++firstchar;
 		if(
@@ -359,30 +436,34 @@ void Cbuf_Execute (cmd_state_t *cmd)
 			(strncmp(firstchar, "in_bind", 7) || !ISWHITESPACE(firstchar[7]))
 		)
 		{
-			if(Cmd_PreprocessString( cmd, line, preprocessed, sizeof(preprocessed), NULL ))
-				Cmd_ExecuteString (cmd, preprocessed, src_command, false);
+			if(Cmd_PreprocessString( current->source, current->text, preprocessed, sizeof(preprocessed), NULL ))
+				Cmd_ExecuteString ( current->source, preprocessed, src_command, false);
 		}
 		else
 		{
-			Cmd_ExecuteString (cmd, line, src_command, false);
+			Cmd_ExecuteString (current->source, current->text, src_command, false);
 		}
 
-		if (cmd->wait)
+		cbuf->size -= strlen(current->text);
+		Z_Free(current);
+
+		if (cbuf->wait)
 		{	// skip out while text still remains in buffer, leaving it
 			// for next frame
-			cmd->wait = false;
-			break;
+			cbuf->wait = false;
+			return;
 		}
 	}
+	cbuf->end = NULL;
 }
 
-void Cbuf_Frame(cmd_state_t *cmd)
+void Cbuf_Frame(cbuf_t *cbuf)
 {
-	Cbuf_Execute_Deferred(cmd);
-	if (cmd->text.cursize)
+	//Cbuf_Execute_Deferred(cmd);
+	if (cbuf->size)
 	{
 		SV_LockThreadMutex();
-		Cbuf_Execute(cmd);
+		Cbuf_Execute(cbuf);
 		SV_UnlockThreadMutex();
 	}
 }
@@ -491,9 +572,6 @@ static void Cmd_Exec(cmd_state_t *cmd, const char *filename)
 	if (isdefaultcfg)
 		Cbuf_InsertText(cmd, "\ncvar_lockdefaults\n");
 
-	// insert newline after the text to make sure the last line is terminated (some text editors omit the trailing newline)
-	// (note: insertion order here is backwards from execution order, so this adds it after the text, by calling it before...)
-	Cbuf_InsertText (cmd, "\n");
 	Cbuf_InsertText (cmd, f);
 	Mem_Free(f);
 
@@ -1497,14 +1575,17 @@ Cmd_Init
 void Cmd_Init(void)
 {
 	cmd_iter_t *cmd_iter;
+	cbuf_t *cbuf = (cbuf_t *)Z_Malloc(sizeof(cbuf_t));
+	cbuf->maxsize = 655360;
+	cbuf->lock = Thread_CreateMutex();
+	host.cbuf = cbuf;
+
 	for (cmd_iter = cmd_iter_all; cmd_iter->cmd; cmd_iter++)
 	{
 		cmd_state_t *cmd = cmd_iter->cmd;
 		cmd->mempool = Mem_AllocPool("commands", 0, NULL);
 		// space for commands and script files
-		cmd->text.data = cmd->text_buf;
-		cmd->text.maxsize = sizeof(cmd->text_buf);
-		cmd->text.cursize = 0;
+		cmd->cbuf = cbuf;
 		cmd->null_string = "";
 	}
 	// client console can see server cvars because the user may start a server
@@ -1514,7 +1595,6 @@ void Cmd_Init(void)
 	cmd_client.auto_flags = CMD_SERVER_FROM_CLIENT;
 	cmd_client.auto_function = CL_ForwardToServer_f; // FIXME: Move this to the client.
 	cmd_client.userdefined = &cmd_userdefined_all;
-	cmd_client.text_mutex = Thread_CreateMutex();
 	// dedicated server console can only see server cvars, there is no client
 	cmd_server.cvars = &cvars_all;
 	cmd_server.cvars_flagsmask = CVAR_SERVER;
@@ -1522,7 +1602,6 @@ void Cmd_Init(void)
 	cmd_server.auto_flags = 0;
 	cmd_server.auto_function = NULL;
 	cmd_server.userdefined = &cmd_userdefined_all;
-	cmd_server.text_mutex = Thread_CreateMutex();
 	// server commands received from clients have no reason to access cvars, cvar expansion seems perilous.
 	cmd_serverfromclient.cvars = &cvars_null;
 	cmd_serverfromclient.cvars_flagsmask = 0;
@@ -1530,7 +1609,6 @@ void Cmd_Init(void)
 	cmd_serverfromclient.auto_flags = 0;
 	cmd_serverfromclient.auto_function = NULL;
 	cmd_serverfromclient.userdefined = &cmd_userdefined_null;
-	cmd_serverfromclient.text_mutex = Thread_CreateMutex();
 
 //
 // register our commands
@@ -1588,7 +1666,7 @@ void Cmd_Shutdown(void)
 		if (cmd->text_mutex)
 		{
 			// we usually have this locked when we get here from Host_Quit_f
-			Cbuf_Unlock(cmd);
+			Cbuf_Unlock(cmd->cbuf);
 		}
 
 		Mem_FreePool(&cmd->mempool);
@@ -2047,7 +2125,7 @@ void Cmd_ExecuteString (cmd_state_t *cmd, const char *text, cmd_source_t src, qb
 	cmd_function_t *func;
 	cmdalias_t *a;
 	if (lockmutex)
-		Cbuf_Lock(cmd);
+		Cbuf_Lock(cmd->cbuf);
 	oldpos = cmd->tokenizebufferpos;
 	cmd->source = src;
 
@@ -2117,7 +2195,7 @@ void Cmd_ExecuteString (cmd_state_t *cmd, const char *text, cmd_source_t src, qb
 done:
 	cmd->tokenizebufferpos = oldpos;
 	if (lockmutex)
-		Cbuf_Unlock(cmd);
+		Cbuf_Unlock(cmd->cbuf);
 }
 
 /*
diff --git a/cmd.h b/cmd.h
index 387b24ff..00077eea 100644
--- a/cmd.h
+++ b/cmd.h
@@ -123,6 +123,8 @@ typedef struct cmd_state_s
 	const char *args;
 	cmd_source_t source;
 
+	struct cbuf_s *cbuf;
+
 	cmd_userdefined_t *userdefined; // possible csqc functions and aliases to execute
 
 	cmd_function_t *engine_functions;
@@ -144,6 +146,23 @@ typedef struct cmd_state_s
 }
 cmd_state_t;
 
+typedef struct cbuf_cmd_s
+{
+	struct cbuf_cmd_s *prev, *next;
+	cmd_state_t *source;
+	char *text;
+	double defer;
+} cbuf_cmd_t;
+
+typedef struct cbuf_s
+{
+	cbuf_cmd_t *start, *end;
+	qboolean wait;
+	size_t maxsize;
+	size_t size;
+	void *lock;
+} cbuf_t;
+
 extern cmd_userdefined_t cmd_userdefined_all; // aliases and csqc functions
 extern cmd_userdefined_t cmd_userdefined_null; // intentionally empty
 
@@ -159,8 +178,8 @@ extern cmd_state_t cmd_serverfromclient;
 
 extern qboolean host_stuffcmdsrun;
 
-void Cbuf_Lock(cmd_state_t *cmd);
-void Cbuf_Unlock(cmd_state_t *cmd);
+void Cbuf_Lock(cbuf_t *cbuf);
+void Cbuf_Unlock(cbuf_t *cbuf);
 
 /*! as new commands are generated from the console or keybindings,
  * the text is added to the end of the command buffer.
@@ -178,9 +197,9 @@ void Cbuf_InsertText (cmd_state_t *cmd, const char *text);
  * Normally called once per frame, but may be explicitly invoked.
  * \note Do not call inside a command function!
  */
-void Cbuf_Execute (cmd_state_t *cmd);
+void Cbuf_Execute (cbuf_t *cbuf);
 /*! Performs deferred commands and runs Cbuf_Execute, called by Host_Frame */
-void Cbuf_Frame (cmd_state_t *cmd);
+void Cbuf_Frame (cbuf_t *cbuf);
 
 //===========================================================================
 
diff --git a/host.c b/host.c
index faadf054..35d84307 100644
--- a/host.c
+++ b/host.c
@@ -316,7 +316,7 @@ static void Host_AddConfigText(cmd_state_t *cmd)
 		Cbuf_InsertText(cmd, "alias startmap_sp \"map start\"\nalias startmap_dm \"map start\"\nexec teu.rc\n");
 	else
 		Cbuf_InsertText(cmd, "alias startmap_sp \"map start\"\nalias startmap_dm \"map start\"\nexec " STARTCONFIGFILENAME "\n");
-	Cbuf_Execute(cmd);
+	Cbuf_Execute(cmd->cbuf);
 }
 
 /*
@@ -403,11 +403,8 @@ double Host_Frame(double time)
 
 	// process console commands
 //		R_TimeReport("preconsole");
-	Cbuf_Frame(&cmd_client);
-	Cbuf_Frame(&cmd_server);
 
-	if(sv.active)
-		Cbuf_Frame(&cmd_serverfromclient);
+	Cbuf_Frame(host.cbuf);
 
 //		R_TimeReport("console");
 
@@ -736,14 +733,14 @@ static void Host_Init (void)
 	if (!FS_FileExists("quake.rc"))
 	{
 		Cbuf_InsertText(cmd, "exec default.cfg\nexec " CONFIGFILENAME "\nexec autoexec.cfg\n");
-		Cbuf_Execute(cmd);
+		Cbuf_Execute(cmd->cbuf);
 	}
 
 	host.state = host_active;
 
 	// run stuffcmds now, deferred previously because it can crash if a server starts that early
 	Cbuf_AddText(cmd,"stuffcmds\n");
-	Cbuf_Execute(cmd);
+	Cbuf_Execute(cmd->cbuf);
 
 	Log_Start();
 
@@ -757,7 +754,7 @@ static void Host_Init (void)
 	if (!sv.active && !cls.demoplayback && !cls.connect_trying)
 	{
 		Cbuf_AddText(&cmd_client, va(vabuf, sizeof(vabuf), "timedemo %s\n", sys.argv[i + 1]));
-		Cbuf_Execute(&cmd_client);
+		Cbuf_Execute((&cmd_client)->cbuf);
 	}
 
 	// check for special demo mode
@@ -767,7 +764,7 @@ static void Host_Init (void)
 	if (!sv.active && !cls.demoplayback && !cls.connect_trying)
 	{
 		Cbuf_AddText(&cmd_client, va(vabuf, sizeof(vabuf), "playdemo %s\n", sys.argv[i + 1]));
-		Cbuf_Execute(&cmd_client);
+		Cbuf_Execute((&cmd_client)->cbuf);
 	}
 
 #ifdef CONFIG_VIDEO_CAPTURE
@@ -777,7 +774,7 @@ static void Host_Init (void)
 	if (!sv.active && !cls.demoplayback && !cls.connect_trying)
 	{
 		Cbuf_AddText(&cmd_client, va(vabuf, sizeof(vabuf), "playdemo %s\ncl_capturevideo 1\n", sys.argv[i + 1]));
-		Cbuf_Execute(&cmd_client);
+		Cbuf_Execute((&cmd_client)->cbuf);
 	}
 #endif
 
@@ -785,7 +782,7 @@ static void Host_Init (void)
 	if (!sv.active && !cls.demoplayback && !cls.connect_trying)
 	{
 		Cbuf_AddText(&cmd_client, "startmap_dm\n");
-		Cbuf_Execute(&cmd_client);
+		Cbuf_Execute((&cmd_client)->cbuf);
 	}
 
 	if (!sv.active && !cls.demoplayback && !cls.connect_trying)
@@ -793,7 +790,7 @@ static void Host_Init (void)
 #ifdef CONFIG_MENU
 		Cbuf_AddText(&cmd_client, "togglemenu 1\n");
 #endif
-		Cbuf_Execute(&cmd_client);
+		Cbuf_Execute((&cmd_client)->cbuf);
 	}
 
 	Con_DPrint("========Initialized=========\n");
diff --git a/quakedef.h b/quakedef.h
index 207c05f6..8c0ba38c 100644
--- a/quakedef.h
+++ b/quakedef.h
@@ -548,6 +548,7 @@ typedef struct host_s
 	double sleeptime; // time spent sleeping overall
 	qboolean restless; // don't sleep
 	qboolean paused; // global paused state, pauses both client and server
+	cbuf_t *cbuf;
 } host_t;
 
 extern host_t host;
diff --git a/sys_shared.c b/sys_shared.c
index 0f7e0bdd..468037be 100644
--- a/sys_shared.c
+++ b/sys_shared.c
@@ -48,10 +48,10 @@ char *Sys_TimeString(const char *timeformat)
 void Sys_Quit (int returnvalue)
 {
 	// Unlock mutexes because the quit command may jump directly here, causing a deadlock
-	if (cmd_client.text_mutex)
-		Cbuf_Unlock(&cmd_client);
-	if (cmd_server.text_mutex)
-		Cbuf_Unlock(&cmd_server);
+	if ((&cmd_client)->cbuf->lock)
+		Cbuf_Unlock((&cmd_client)->cbuf);
+	if ((&cmd_server)->cbuf->lock)
+		Cbuf_Unlock((&cmd_server)->cbuf);
 	SV_UnlockThreadMutex();
 	TaskQueue_Frame(true);