Re: [Qemu-devel] [PATCH v9 01/27] gdbstub: Add infrastructure to parse cmd packets

2019-05-14 Thread Alex Bennée


Jon Doron  writes:

> Signed-off-by: Jon Doron 
> ---

> +
> +/*
> + * cmd_startswith -> cmd is compared using startswith
> + *
> + *
> + * schema definitions:
> + * Each schema parameter entry consists of 2 chars,
> + * the first char represents the parameter type handling
> + * the second char represents the delimiter for the next parameter
> + *
> + * Currently supported schema types:
> + * 'l' -> unsigned long (stored in .val_ul)
> + * 'L' -> unsigned long long (stored in .val_ull)
> + * 's' -> string (stored in .data)
> + * 'o' -> single char (stored in .opcode)
> + * 't' -> thread id (stored in .thread_id)
> + * '?' -> skip according to delimiter
> + *
> + * Currently supported delimiters:
> + * '?' -> Stop at any delimiter (",;:=\0")
> + * '0' -> Stop at "\0"
> + * '.' -> Skip 1 char unless reached "\0"
> + * Any other value is treated as the delimiter value itself
> + */
> +typedef struct GdbCmdParseEntry {
> +GdbCmdHandler handler;
> +const char *cmd;
> +union {
> +int flags;
> +struct {
> +int cmd_startswith:1;
> +};
> +};

This union seems a little over the top given flags isn't used AFAICT.
Why not just have a bool cmd_startswith for now? You can always expand
the structure later if you need to.

Otherwise:

Reviewed-by: Alex Bennée 

--
Alex Bennée



[Qemu-devel] [PATCH v9 01/27] gdbstub: Add infrastructure to parse cmd packets

2019-05-02 Thread Jon Doron
Signed-off-by: Jon Doron 
---
 gdbstub.c | 200 ++
 1 file changed, 200 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index d54abd17cc..d5e0f3878a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1268,6 +1268,206 @@ out:
 return res;
 }
 
+typedef union GdbCmdVariant {
+const char *data;
+uint8_t opcode;
+unsigned long val_ul;
+unsigned long long val_ull;
+struct {
+GDBThreadIdKind kind;
+uint32_t pid;
+uint32_t tid;
+} thread_id;
+} GdbCmdVariant;
+
+static const char *cmd_next_param(const char *param, const char delimiter)
+{
+static const char all_delimiters[] = ",;:=";
+char curr_delimiters[2] = {0};
+const char *delimiters;
+
+if (delimiter == '?') {
+delimiters = all_delimiters;
+} else if (delimiter == '0') {
+return strchr(param, '\0');
+} else if (delimiter == '.' && *param) {
+return param + 1;
+} else {
+curr_delimiters[0] = delimiter;
+delimiters = curr_delimiters;
+}
+
+param += strcspn(param, delimiters);
+if (*param) {
+param++;
+}
+return param;
+}
+
+static int cmd_parse_params(const char *data, const char *schema,
+GdbCmdVariant *params, int *num_params)
+{
+int curr_param;
+const char *curr_schema, *curr_data;
+
+*num_params = 0;
+
+if (!schema) {
+return 0;
+}
+
+curr_schema = schema;
+curr_param = 0;
+curr_data = data;
+while (curr_schema[0] && curr_schema[1] && *curr_data) {
+switch (curr_schema[0]) {
+case 'l':
+if (qemu_strtoul(curr_data, _data, 16,
+ [curr_param].val_ul)) {
+return -EINVAL;
+}
+curr_param++;
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+case 'L':
+if (qemu_strtou64(curr_data, _data, 16,
+  (uint64_t *)[curr_param].val_ull)) {
+return -EINVAL;
+}
+curr_param++;
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+case 's':
+params[curr_param].data = curr_data;
+curr_param++;
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+case 'o':
+params[curr_param].opcode = *(uint8_t *)curr_data;
+curr_param++;
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+case 't':
+params[curr_param].thread_id.kind =
+read_thread_id(curr_data, _data,
+   [curr_param].thread_id.pid,
+   [curr_param].thread_id.tid);
+curr_param++;
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+case '?':
+curr_data = cmd_next_param(curr_data, curr_schema[1]);
+break;
+default:
+return -EINVAL;
+}
+curr_schema += 2;
+}
+
+*num_params = curr_param;
+return 0;
+}
+
+typedef struct GdbCmdContext {
+GDBState *s;
+GdbCmdVariant *params;
+int num_params;
+uint8_t mem_buf[MAX_PACKET_LENGTH];
+char str_buf[MAX_PACKET_LENGTH + 1];
+} GdbCmdContext;
+
+typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
+
+/*
+ * cmd_startswith -> cmd is compared using startswith
+ *
+ *
+ * schema definitions:
+ * Each schema parameter entry consists of 2 chars,
+ * the first char represents the parameter type handling
+ * the second char represents the delimiter for the next parameter
+ *
+ * Currently supported schema types:
+ * 'l' -> unsigned long (stored in .val_ul)
+ * 'L' -> unsigned long long (stored in .val_ull)
+ * 's' -> string (stored in .data)
+ * 'o' -> single char (stored in .opcode)
+ * 't' -> thread id (stored in .thread_id)
+ * '?' -> skip according to delimiter
+ *
+ * Currently supported delimiters:
+ * '?' -> Stop at any delimiter (",;:=\0")
+ * '0' -> Stop at "\0"
+ * '.' -> Skip 1 char unless reached "\0"
+ * Any other value is treated as the delimiter value itself
+ */
+typedef struct GdbCmdParseEntry {
+GdbCmdHandler handler;
+const char *cmd;
+union {
+int flags;
+struct {
+int cmd_startswith:1;
+};
+};
+const char *schema;
+} GdbCmdParseEntry;
+
+static inline int startswith(const char *string, const char *pattern)
+{
+  return !strncmp(string, pattern, strlen(pattern));
+}
+
+static int process_string_cmd(
+GDBState *s, void *user_ctx, const char *data,
+const GdbCmdParseEntry *cmds, int num_cmds)
+__attribute__((unused));
+
+static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
+  const GdbCmdParseEntry *cmds, int num_cmds)
+{
+int i, schema_len,