From: bones_was_here Date: Tue, 26 Sep 2023 04:40:22 +0000 (+1000) Subject: cbuf: refactor and fix parsing of text blocks into the linked list X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=c4cba77a1452975e9e8482a2df29cac4259c418f;p=xonotic%2Fdarkplaces.git cbuf: refactor and fix parsing of text blocks into the linked list Simplifies the parsing to a single loop with less state tracking. Reorganises the code for readability. Fixes https://gitlab.com/xonotic/darkplaces/-/issues/378 Signed-off-by: bones_was_here --- diff --git a/cmd.c b/cmd.c index 0fd60018..deb9a4a6 100644 --- a/cmd.c +++ b/cmd.c @@ -74,7 +74,7 @@ Cmd_Defer_f Cause a command to be executed after a delay. ============ */ -static cmd_input_t *Cbuf_LinkGet(cmd_buf_t *cbuf, cmd_input_t *existing); +static cmd_input_t *Cbuf_NodeGet(cmd_buf_t *cbuf, cmd_input_t *existing); static void Cmd_Defer_f (cmd_state_t *cmd) { cmd_input_t *current; @@ -98,7 +98,7 @@ static void Cmd_Defer_f (cmd_state_t *cmd) else if(Cmd_Argc(cmd) == 3) { const char *text = Cmd_Argv(cmd, 2); - current = Cbuf_LinkGet(cbuf, NULL); + current = Cbuf_NodeGet(cbuf, NULL); current->length = strlen(text); current->source = cmd; current->delay = atof(Cmd_Argv(cmd, 1)); @@ -169,182 +169,135 @@ static void Cmd_Centerprint_f (cmd_state_t *cmd) COMMAND BUFFER + * The Quake command-line is super basic. It can be entered in the console + * or in config files. A semicolon is used to terminate a command and chain + * them together. Otherwise, a newline delineates command input. + * + * In most engines, the Quake command-line is a simple linear text buffer that + * is parsed when it executes. In Darkplaces, we use a linked list of command + * input and parse the input on the spot. + * + * This was done because Darkplaces allows multiple command interpreters on the + * same thread. Previously, each interpreter maintained its own buffer and this + * caused problems related to execution order, and maintaining a single simple + * buffer for all interpreters makes it non-trivial to keep track of which + * command should execute on which interpreter. + ============================================================================= */ -static cmd_input_t *Cbuf_LinkGet(cmd_buf_t *cbuf, cmd_input_t *existing) +/* +============ +Cbuf_NodeGet + +Returns an existing buffer node for appending or reuse, or allocates a new one +============ +*/ +static cmd_input_t *Cbuf_NodeGet(cmd_buf_t *cbuf, cmd_input_t *existing) { - cmd_input_t *ret = NULL; + cmd_input_t *node; if(existing && existing->pending) - ret = existing; + node = existing; else if(!List_Is_Empty(&cbuf->free)) { - ret = List_Entry(cbuf->free.next, cmd_input_t, list); - ret->length = 0; - ret->pending = false; + node = List_Entry(cbuf->free.next, cmd_input_t, list); + node->length = node->pending = 0; + } + else + { + node = (cmd_input_t *)Mem_Alloc(cbuf_mempool, sizeof(cmd_input_t)); + node->list.prev = node->list.next = &node->list; + node->size = node->length = node->pending = 0; } - return ret; -} - -static cmd_input_t *Cmd_AllocInputNode(void) -{ - cmd_input_t *node = (cmd_input_t *)Mem_Alloc(cbuf_mempool, sizeof(cmd_input_t)); - node->list.prev = node->list.next = &node->list; - node->size = node->length = node->pending = 0; return node; } +/* +============ +Cbuf_LinkString -// Cloudwalk FIXME: The entire design of this thing is overly complicated. -// We could very much safely have one input node per line whether or not -// the command was terminated. We don't need to split up input nodes per command -// executed. -static size_t Cmd_ParseInput (cmd_input_t **output, char **input) +Copies a command string into a buffer node +============ +*/ +static void Cbuf_LinkString(cmd_state_t *cmd, llist_t *head, cmd_input_t *existing, const char *text, qbool leavepending, unsigned int cmdsize) { - size_t pos, cmdsize = 0, start = 0; - qbool command = false, lookahead = false; - qbool quotes = false, comment = false; - qbool escaped = false; - - /* - * The Quake command-line is super basic. It can be entered in the console - * or in config files. A semicolon is used to terminate a command and chain - * them together. Otherwise, a newline delineates command input. - * - * In most engines, the Quake command-line is a simple linear text buffer that - * is parsed when it executes. In Darkplaces, we use a linked list of command - * input and parse the input on the spot. - * - * This was done because Darkplaces allows multiple command interpreters on the - * same thread. Previously, each interpreter maintained its own buffer and this - * caused problems related to execution order, and maintaining a single simple - * buffer for all interpreters makes it non-trivial to keep track of which - * command should execute on which interpreter. - */ - - // Run until command and lookahead are both true, or until we run out of input. - for (pos = 0; (*input)[pos]; pos++) - { - // Look for newlines and semicolons. Ignore semicolons in quotes. - switch((*input)[pos]) - { - case '\r': - case '\n': - command = false; - comment = false; - break; - default: - if(!comment) // Not a newline so far. Still not a valid command yet. - { - if(!quotes && (*input)[pos] == ';') // Ignore semicolons in quotes. - command = false; - else if (ISCOMMENT((*input), pos)) // Comments - { - comment = true; - command = false; - } - else - { - command = true; - if(!lookahead) - { - if(!cmdsize) - start = pos; - cmdsize++; - } - - switch((*input)[pos]) - { - case '"': - if (!escaped) - quotes = !quotes; - else - escaped = false; - break; - case '\\': - if (!escaped && quotes) - escaped = true; - else if (escaped) - escaped = false; - break; - } - } - } - } - if(cmdsize && !command) - lookahead = true; + cmd_buf_t *cbuf = cmd->cbuf; + cmd_input_t *node = Cbuf_NodeGet(cbuf, existing); + unsigned int offset = node->length; // > 0 if(pending) - if(command && lookahead) - break; + // node will match existing if its text was pending continuation + if(node != existing) + { + node->source = cmd; + List_Move_Tail(&node->list, head); } - if(cmdsize) + node->length += cmdsize; + if(node->size < node->length) { - size_t offset = 0; - - if(!*output) - *output = Cmd_AllocInputNode(); - - // Append, since this input line hasn't closed yet. - if((*output)->pending) - offset = (*output)->length; - - (*output)->length += cmdsize; - - if((*output)->size < (*output)->length) - { - (*output)->text = (char *)Mem_Realloc(cbuf_mempool, (*output)->text, (*output)->length + 1); - (*output)->size = (*output)->length; - } - - strlcpy(&(*output)->text[offset], &(*input)[start], cmdsize + 1); - - /* - * If we were still looking ahead by the time we broke from the loop, the command input - * hasn't terminated yet and we're still expecting more, so keep this node open for appending later. - */ - (*output)->pending = !lookahead; + node->text = (char *)Mem_Realloc(cbuf_mempool, node->text, node->length + 1); + node->size = node->length; } + cbuf->size += cmdsize; - // Set input to its new position. Can be NULL. - *input = &(*input)[pos]; - - return cmdsize; + strlcpy(&node->text[offset], text, cmdsize + 1); // always sets the last char to \0 + //Con_Printf("^5Cbuf_LinkString(): %s `^7%s^5`\n", node->pending ? "append" : "new", &node->text[offset]); + node->pending = leavepending; } -// Cloudwalk: Not happy with this, but it works. -static void Cbuf_LinkCreate(cmd_state_t *cmd, llist_t *head, cmd_input_t *existing, const char *text) +/* +============ +Cbuf_ParseText + +Parses text to isolate command strings for linking into the buffer +separators: \n \r or unquoted and uncommented ';' +============ +*/ +static void Cbuf_ParseText(cmd_state_t *cmd, llist_t *head, cmd_input_t *existing, const char *text, qbool allowpending) { - char *in = (char *)&text[0]; - cmd_buf_t *cbuf = cmd->cbuf; - size_t totalsize = 0, newsize = 0; - cmd_input_t *current = NULL; + unsigned int cmdsize = 0, start = 0, pos; + qbool quotes = false, comment = false; - // Slide the pointer down until we reach the end - while(*in) + for (pos = 0; text[pos]; ++pos) { - // Check if the current node is still accepting input (input line hasn't terminated) - current = Cbuf_LinkGet(cbuf, existing); - newsize = Cmd_ParseInput(¤t, &in); - - // Valid command - if(newsize) + switch(text[pos]) { - // current will match existing if the input line hasn't terminated yet - if(current != existing) - { - current->source = cmd; - List_Move_Tail(¤t->list, head); - } + case ';': + if (comment || quotes) + break; + case '\r': + case '\n': + comment = false; + quotes = false; // matches div0-stable + if (cmdsize) + { + Cbuf_LinkString(cmd, head, existing, &text[start], false, cmdsize); + cmdsize = 0; + } + else if (existing && existing->pending) // all I got was this lousy \n + existing->pending = false; + continue; // don't increment cmdsize + + case '/': + if (!quotes && text[pos + 1] == '/' && (pos == 0 || ISWHITESPACE(text[pos - 1]))) + comment = true; + break; + case '"': + if (!comment && (pos == 0 || text[pos - 1] != '\\')) + quotes = !quotes; + break; + } - totalsize += newsize; + if (!comment) + { + if (!cmdsize) + start = pos; + ++cmdsize; } - else if (current == existing && !totalsize) - current->pending = false; - current = NULL; } - cbuf->size += totalsize; + if (cmdsize) // the line didn't end yet but we do have a string + Cbuf_LinkString(cmd, head, existing, &text[start], allowpending, cmdsize); } /* @@ -366,9 +319,9 @@ void Cbuf_AddText (cmd_state_t *cmd, const char *text) Con_Print("Cbuf_AddText: overflow\n"); else { - Cbuf_LinkCreate(cmd, &llist, (List_Is_Empty(&cbuf->start) ? NULL : List_Entry(cbuf->start.prev, cmd_input_t, list)), text); - if(!List_Is_Empty(&llist)) - List_Splice_Tail(&llist, &cbuf->start); + // If the string terminates but the (last) line doesn't, the node will be left in the pending state (to be continued). + Cbuf_ParseText(cmd, &llist, (List_Is_Empty(&cbuf->start) ? NULL : List_Entry(cbuf->start.prev, cmd_input_t, list)), text, true); + List_Splice_Tail(&llist, &cbuf->start); } Cbuf_Unlock(cbuf); } @@ -378,7 +331,6 @@ void Cbuf_AddText (cmd_state_t *cmd, const char *text) Cbuf_InsertText Adds command text immediately after the current command -FIXME: actually change the command buffer to do less copying ============ */ void Cbuf_InsertText (cmd_state_t *cmd, const char *text) @@ -389,14 +341,15 @@ void Cbuf_InsertText (cmd_state_t *cmd, const char *text) Cbuf_Lock(cbuf); - // we need to memmove the existing text and stuff this in before it... if (cbuf->size + l >= cbuf->maxsize) Con_Print("Cbuf_InsertText: overflow\n"); else { - Cbuf_LinkCreate(cmd, &llist, (List_Is_Empty(&cbuf->start) ? NULL : List_Entry(cbuf->start.next, cmd_input_t, list)), text); - if(!List_Is_Empty(&llist)) - List_Splice(&llist, &cbuf->start); + // bones_was_here assertion: when prepending to the buffer it never makes sense to leave node(s) in the `pending` state, + // it would have been impossible to append to such text later in the old raw text buffer, + // and allowing it causes bugs when .cfg files lack \n at EOF (see: https://gitlab.com/xonotic/darkplaces/-/issues/378). + Cbuf_ParseText(cmd, &llist, (List_Is_Empty(&cbuf->start) ? NULL : List_Entry(cbuf->start.next, cmd_input_t, list)), text, false); + List_Splice(&llist, &cbuf->start); } Cbuf_Unlock(cbuf); @@ -1654,7 +1607,7 @@ void Cmd_Init(void) cmd_buf_t *cbuf; cbuf_mempool = Mem_AllocPool("Command buffer", 0, NULL); cbuf = (cmd_buf_t *)Mem_Alloc(cbuf_mempool, sizeof(cmd_buf_t)); - cbuf->maxsize = 655360; + cbuf->maxsize = CMDBUFSIZE; cbuf->lock = Thread_CreateMutex(); cbuf->wait = false; host.cbuf = cbuf;