Make command parsing more robust

cmdbuf.c

@@ -0,0 +1,71 @@
+#include <cmdbuf.h>
+#include <string.h>
+
+bool cmd_match_str(CommandBuf *cmd, const char *str) {
+    size_t len = strlen(str);
+    if (cmd->len < len) return false;
+    if (!strncmp(cmd->buf, str, len)) {
+        cmd->buf += len;
+        cmd->len -= len;
+        return true;
+    }
+    return false;
+}
+
+bool cmd_match_only_str(CommandBuf *cmd, const char *str) {
+    size_t len = strlen(str);
+    if (cmd->len != len) return false;
+    if (!strncmp(cmd->buf, str, len)) {
+        cmd->buf += len;
+        cmd->len -= len;
+        return true;
+    }
+    return false;
+}
+
+static bool parse_hex_digit(char digit, char *out) {
+    if (digit >= '0' && digit <= '9') {
+        *out = digit - '0';
+        return true;
+    }
+    if (digit >= 'a' && digit <= 'f') {
+        *out = digit - 'a' + 10;
+        return true;
+    }
+    if (digit >= 'A' && digit <= 'F') {
+        *out = digit - 'A' + 10;
+        return true;
+    }
+    return false;
+}
+
+bool cmd_match_hex_number(CommandBuf *cmd, uint32_t *value) {
+    size_t read = 0;
+    size_t max_len = cmd->len;
+    if (max_len > 8) max_len = 8;
+
+    *value = 0;
+    for (; read < max_len; ++read) {
+        char digit;
+        if (!parse_hex_digit(cmd->buf[read], &digit)) break;
+        *value = (*value << 4) | digit;
+    }
+    if (!read) return false;
+
+    cmd->buf += read;
+    cmd->len -= read;
+    return true;
+}
+
+bool cmd_match_hex_bytes(CommandBuf *cmd, const uint32_t count, char *value) {
+    if (cmd->len < (count * 2)) return false;
+    for (size_t i = 0; i < count; ++i) {
+        char hi, lo;
+        if (!parse_hex_digit(cmd->buf[i * 2], &hi)) return false;
+        if (!parse_hex_digit(cmd->buf[(i * 2) + 1], &lo)) return false;
+        value[i] = (hi << 4) | lo;
+    }
+    cmd->buf += (count * 2);
+    cmd->len -= (count * 2);
+    return true;
+}

include/cmdbuf.h

@@ -0,0 +1,22 @@
+#ifndef RDBSERVER_CMDBUF_H_
+#define RDBSERVER_CMDBUF_H_
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+typedef struct CommandBuf {
+    char *buf;
+    size_t len;
+} CommandBuf;
+
+/* Try to consume a string literal. */
+bool cmd_match_str(CommandBuf *cmd, const char *str);
+/* Try to consume a string literal, if it is the only thing left in the buffer. */
+bool cmd_match_only_str(CommandBuf *cmd, const char *str);
+/* Try to consume a base-16 number, and return the value. */
+bool cmd_match_hex_number(CommandBuf *cmd, uint32_t *value);
+/* Try to consume a hex-encoded list of bytes, and return the value. */
+bool cmd_match_hex_bytes(CommandBuf *cmd, const uint32_t count, char *value);
+
+#endif

main.c

@@ -1,4 +1,5 @@
 #include <client.h>
+#include <cmdbuf.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -74,134 +75,74 @@ const uint32_t SYSTEM_REGISTERS[] = {
     31,
 };
 
-const char PC_INDEX = 32 + 13;
+const uint32_t PC_INDEX = 32 + 13;
 
-bool read_hex_digit(char digit, char *out) {
+int handle_command(RdbClient *client, CommandBuf *cmd, VB *sim) {
-    if (digit >= '0' && digit <= '9') {
-        *out = digit - '0';
-        return true;
-    }
-    if (digit >= 'a' && digit <= 'f') {
-        *out = digit - 'a' + 10;
-        return true;
-    }
-    if (digit >= 'A' && digit <= 'F') {
-        *out = digit - 'A' + 10;
-        return true;
-    }
-    return false;
-}
-
-bool read_hex(char *buf, int len, int *out) {
-    *out = 0;
-    for (int i = 0; i < len; ++i) {
-        char outdigit;
-        if (!read_hex_digit(buf[i], &outdigit)) return false;
-        *out = (*out << 4) | (int) outdigit;
-    }
-    return true;
-}
-
-bool read_hex_byte(char *buf, char *val) {
-    char digit1, digit2;
-    if (!read_hex_digit(buf[0], &digit1) || !read_hex_digit(buf[1], &digit2)) {
-        return false;
-    }
-    *val = (digit1 << 4) | digit2;
-    return true;
-}
-
-bool read_hex_i32(char *buf, int32_t *val) {
-    char byte;
-    *val = 0;
-    for (int i = 0; i < 4; ++i) {
-        if (!read_hex_byte(buf + (i * 2), &byte)) {
-            return false;
-        }
-        *val |= ((int32_t) (uint8_t) byte) << (i * 8);
-    }
-    return true;
-}
-
-int handle_command(RdbClient *client, char *cmd, size_t cmdlen, VB *sim) {
     rdb_client_begin_packet(client);
 
-    if (!strncmp(cmd, "\x03", cmdlen)) {
+    if (cmd_match_only_str(cmd, "\x03")) {
         rdb_client_write_str(client, "T05thread:p1.t1;threads:p1.t1");
         return rdb_client_send_packet(client);
     }
-
+    if (cmd_match_only_str(cmd, "QStartNoAckMode")) {
-    if (!strncmp(cmd, "QStartNoAckMode", cmdlen)) {
         client->should_ack = false;
         rdb_client_write_str(client, "OK");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qSupported", 10)) {
+    if (cmd_match_str(cmd, "qSupported")) {
         rdb_client_write_str(client, "no-resumed+;multiprocess;vContSupported;QNonStop+");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "QThreadSuffixSupported", cmdlen)) {
+    if (cmd_match_only_str(cmd, "QThreadSuffixSupported")) {
         rdb_client_write_str(client, "OK");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "QListThreadsInStopReply", cmdlen)) {
+    if (cmd_match_only_str(cmd, "QListThreadsInStopReply")) {
         rdb_client_write_str(client, "OK");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qHostInfo", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qHostInfo")) {
         rdb_client_write_str(client, "triple:");
         rdb_client_write_str_hex(client, "v810-unknown-vb");
         rdb_client_write_str(client, ";endian:little;ptrsize:4;");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qProcessInfo", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qProcessInfo")) {
         rdb_client_write_str(client, "pid:1;triple:");
         rdb_client_write_str_hex(client, "v810-unknown-vb");
         rdb_client_write_str(client, "endian:little;ptrsize:4;");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qRegisterInfo", 13)) {
+    if (cmd_match_str(cmd, "qRegisterInfo")) {
-        char reg;
+        uint32_t reg_no;
-        if (cmdlen == 14) {
+        if (!cmd_match_hex_number(cmd, &reg_no)) return 1;
-            if (!read_hex_digit(cmd[13], &reg)) return 1;
+        if (reg_no <= PC_INDEX) {
-        } else {
+            rdb_client_write_str(client, REGISTERS[reg_no]);
-            if (!read_hex_byte(cmd + 13, &reg)) return 1;
-        }
-        if (reg <= PC_INDEX) {
-            rdb_client_write_str(client, REGISTERS[(size_t) reg]);
         }
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qfThreadInfo", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qfThreadInfo")) {
         rdb_client_write_str(client, "mp1.t1");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qsThreadInfo", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qsThreadInfo")) {
         rdb_client_write_str(client, "l");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "vCont?", cmdlen)) {
+    if (cmd_match_only_str(cmd, "vCont?")) {
         rdb_client_write_str(client, "c;C;s;S");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qVAttachOrWaitSupported", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qVAttachOrWaitSupported")) {
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qC", cmdlen)) {
+    if (cmd_match_only_str(cmd, "qC")) {
         rdb_client_write_str(client, "QCp1.t1");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "p", 1)) {
+    if (cmd_match_str(cmd, "p")) {
-        char reg_no;
+        uint32_t reg_no;
-        if (cmd[2] == ';') {
+        if (!cmd_match_hex_number(cmd, &reg_no)) return 1;
-            if (!read_hex_digit(cmd[1], &reg_no)) {
-                return 1;
-            }
-        } else {
-            if (!read_hex_byte(cmd + 1, &reg_no)) {
-                return 1;
-            }
-        }
         int32_t reg_value;
         if (reg_no == PC_INDEX) {
             reg_value = vbGetProgramCounter(sim);
@@ -213,24 +154,18 @@ int handle_command(RdbClient *client, char *cmd, size_t cmdlen, VB *sim) {
         rdb_client_write_i32_hex(client, reg_value);
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "P", 1)) {
+    if (cmd_match_str(cmd, "P")) {
-        char reg_no;
+        uint32_t reg_no;
-        int32_t reg_value;
+        char reg_bytes[4];
-        if (cmd[2] == '=') {
+        if (!cmd_match_hex_number(cmd, &reg_no)) return -1;
-            if (!read_hex_digit(cmd[1], &reg_no)) {
+        if (!cmd_match_str(cmd, "=")) return -1;
-                return 1;
+        if (!cmd_match_hex_bytes(cmd, 4, reg_bytes)) return -1;
-            }
+
-            if (!read_hex_i32(cmd + 3, &reg_value)) {
+        int32_t reg_value = ((uint32_t) (reg_bytes[3]) << 24) |
-                return 1;
+                            ((uint32_t) (reg_bytes[2]) << 16) |
-            }
+                            ((uint32_t) (reg_bytes[1]) << 8) |
-        } else {
+                            ((uint32_t) reg_bytes[0]);
-            if (!read_hex_byte(cmd + 1, &reg_no)) {
+
-                return 1;
-            }
-            if (!read_hex_i32(cmd + 4, &reg_value)) {
-                return 1;
-            }
-        }
         if (reg_no == PC_INDEX) {
             vbSetProgramCounter(sim, reg_value);
         } else if (reg_no > 31) {
@@ -241,51 +176,40 @@ int handle_command(RdbClient *client, char *cmd, size_t cmdlen, VB *sim) {
         rdb_client_write_str(client, "OK");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "Hc-1", 1)) {
+    if (cmd_match_str(cmd, "Hc-1")) {
         rdb_client_write_str(client, "OK");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "c", cmdlen)) {
+    if (cmd_match_only_str(cmd, "c")) {
         printf("running until we hit a breakpoint or the server stops us\n");
         return 0;
     }
-    if (!strncmp(cmd, "?", cmdlen)) {
+    if (cmd_match_only_str(cmd, "?")) {
         rdb_client_write_str(client, "T00thread:p1.t1;threads:p1.t1;");
         return rdb_client_send_packet(client);
     }
-    if (!strncmp(cmd, "qMemoryRegionInfo:", 18)) {
+    if (cmd_match_str(cmd, "qMemoryRegionInfo:")) {
         rdb_client_write_str(client, "start:0;size:100000000;permissions:rx;name:");
         rdb_client_write_str_hex(client, "ROM");
         rdb_client_write_str(client, ";");
         return rdb_client_send_packet(client);
     }
-    if (*cmd == 'm') {
+    if (cmd_match_str(cmd, "m")) {
-        // all other memory is 0
+        // read memory
-        int commapos = -1;
+        uint32_t address;
-        for (size_t i = 2; i < cmdlen; ++i) {
+        uint32_t len;
-            if (cmd[i] == ',') {
+        if (!cmd_match_hex_number(cmd, &address)) return -1;
-                commapos = (int) i;
+        if (!cmd_match_str(cmd, ",")) return -1;
-                break;
+        if (!cmd_match_hex_number(cmd, &len)) return -1;
-            }
+
-        }
-        if (commapos == -1) {
-            fprintf(stderr, "malformed memory read");
-            return -1;
-        }
-        int address, len;
-        if (!read_hex(cmd + 1, commapos - 1, &address) ||
-            !read_hex(cmd + commapos + 1, cmdlen - commapos - 1, &len)) {
-                fprintf(stderr, "malformed memory read");
-                return -1;
-        }
         printf("read %d bytes from %d\n", len, address);
-        for (int i = 0; i < len; ++i) {
+        for (uint32_t i = 0; i < len; ++i) {
             uint8_t byte = vbRead(sim, address + i, VB_U8);
             rdb_client_write_i8_hex(client, byte);
         }
         return rdb_client_send_packet(client);
     }
-    fprintf(stderr, "Unrecognized command %.*s\n", (int) cmdlen, cmd);
+    fprintf(stderr, "Unrecognized command.");
     return rdb_client_send_packet(client);
 }
 
@@ -306,7 +230,10 @@ int server(int connfd, VB *sim) {
         } else {
             printf("received command \"%.*s\"\n", (int) len, buf);
             fflush(stdout);
-            int res = handle_command(&client, buf, len, sim);
+            CommandBuf cmd;
+            cmd.buf = buf;
+            cmd.len = len;
+            int res = handle_command(&client, &cmd, sim);
             if (res != 0) {
                 return res;
             }

makefile

@@ -1,6 +1,6 @@
 build:
 	@mkdir -p build
-	@gcc main.c client.c ../vbtest/vb.c -I include -I ../vbtest \
+	@gcc main.c client.c cmdbuf.c ../vbtest/vb.c -I include -I ../vbtest \
 	-Werror -Wall -Wextra -Wpedantic \
 	-Wno-unused-parameter -Wno-unused-function \
 	-o ./build/rdb