Replace the series of sscanf in packet_query with a dedicated qXfer request parser function and a lookup into a predefined handler table.
Signed-off-by: Jinoh Kang jinoh.kang.kr@gmail.com ---
Notes: v3 -> v4: - Refactor the code a bit - Revert to scanf() when parsing Xfer packet lines - Fix pointer variable declaration style
programs/winedbg/gdbproxy.c | 167 ++++++++++++++++++++++++++---------- 1 file changed, 121 insertions(+), 46 deletions(-)
diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 4ed99c4c06c..ace3c71492c 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -61,6 +61,14 @@ struct reply_buffer size_t alloc; };
+#define STRINGIFY2(x) #x +#define STRINGIFY(x) STRINGIFY2(x) + +#define MAX_QX_NAME_LEN 31 +#define MAX_QX_ANNEX_LEN 31 +#define QX_NAME_SIZE (MAX_QX_NAME_LEN+1) +#define QX_ANNEX_SIZE (MAX_QX_ANNEX_LEN+1) + struct gdb_context { /* gdb information */ @@ -89,6 +97,8 @@ struct gdb_context /* Unix environment */ ULONG_PTR wine_segs[3]; /* load addresses of the ELF wine exec segments (text, bss and data) */ BOOL no_ack_mode; + int qxfer_object_idx; + char qxfer_object_annex[QX_ANNEX_SIZE]; struct reply_buffer qxfer_buffer; };
@@ -812,7 +822,7 @@ static int addr_width(struct gdb_context* gdbctx) }
enum packet_return {packet_error = 0x00, packet_ok = 0x01, packet_done = 0x02, - packet_last_f = 0x80}; + packet_send_buffer = 0x03, packet_last_f = 0x80};
static void packet_reply_grow(struct gdb_context* gdbctx, size_t size) { @@ -1768,11 +1778,16 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO return TRUE; }
-static void packet_query_libraries(struct gdb_context* gdbctx) +static enum packet_return packet_query_libraries(struct gdb_context* gdbctx) { struct reply_buffer* reply = &gdbctx->qxfer_buffer; BOOL opt;
+ if (!gdbctx->process) return packet_error; + + if (gdbctx->qxfer_object_annex[0]) + return packet_reply_error(gdbctx, 0); + /* this will resynchronize builtin dbghelp's internal ELF module list */ SymLoadModule(gdbctx->process->handle, 0, 0, 0, 0, 0);
@@ -1781,14 +1796,21 @@ static void packet_query_libraries(struct gdb_context* gdbctx) SymEnumerateModules64(gdbctx->process->handle, packet_query_libraries_cb, gdbctx); SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, opt); reply_buffer_append_str(reply, "</library-list>"); + + return packet_send_buffer; }
-static void packet_query_threads(struct gdb_context* gdbctx) +static enum packet_return packet_query_threads(struct gdb_context* gdbctx) { struct reply_buffer* reply = &gdbctx->qxfer_buffer; struct dbg_process* process = gdbctx->process; struct dbg_thread* thread;
+ if (!process) return packet_error; + + if (gdbctx->qxfer_object_annex[0]) + return packet_reply_error(gdbctx, 0); + reply_buffer_append_str(reply, "<threads>"); LIST_FOR_EACH_ENTRY(thread, &process->threads, struct dbg_thread, entry) { @@ -1800,11 +1822,12 @@ static void packet_query_threads(struct gdb_context* gdbctx) reply_buffer_append_str(reply, ""/>"); } reply_buffer_append_str(reply, "</threads>"); + + return packet_send_buffer; }
-static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_cpu* cpu) +static void packet_query_target_xml(struct gdb_context* gdbctx, struct reply_buffer* reply, struct backend_cpu* cpu) { - struct reply_buffer* reply = &gdbctx->qxfer_buffer; const char* feature_prefix = NULL; const char* feature = NULL; char buffer[256]; @@ -1918,10 +1941,61 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c reply_buffer_append_str(reply, "</target>"); }
+static enum packet_return packet_query_features(struct gdb_context* gdbctx) +{ + struct reply_buffer* reply = &gdbctx->qxfer_buffer; + struct dbg_process* process = gdbctx->process; + + if (!process) return packet_error; + + if (strncmp(gdbctx->qxfer_object_annex, "target.xml", QX_ANNEX_SIZE) == 0) + { + struct backend_cpu *cpu = process->be_cpu; + if (!cpu) return packet_error; + + packet_query_target_xml(gdbctx, reply, cpu); + + return packet_send_buffer; + } + + return packet_reply_error(gdbctx, 0); +} + +struct qxfer +{ + const char* name; + enum packet_return (*handler)(struct gdb_context* gdbctx); +} qxfer_handlers[] = +{ + {"libraries", packet_query_libraries}, + {"threads" , packet_query_threads }, + {"features" , packet_query_features }, +}; + +static BOOL parse_xfer_read(const char* ptr, char* name_buf, char* annex_buf, unsigned int* offp, unsigned int* lenp) +{ + int n; + + /* Xfer:<object>:read:<annex>:<offset-hex>,<length-hex> */ + + if (sscanf(ptr, "Xfer:%" STRINGIFY(MAX_QX_NAME_LEN) "[^:]:read:%n", name_buf, &n) != 1) + return FALSE; + ptr += (unsigned int)n; + + if (sscanf(ptr, "%" STRINGIFY(MAX_QX_ANNEX_LEN) "[^:]%n", annex_buf, &n) != 1) + { + annex_buf[0] = '\0'; + n = 0; + } + ptr += (unsigned int)n; + + return sscanf(ptr, ":%x,%x", offp, lenp) == 2; +} + static enum packet_return packet_query(struct gdb_context* gdbctx) { + char object_name[QX_NAME_SIZE], annex[QX_ANNEX_SIZE]; unsigned int off, len; - struct backend_cpu *cpu;
switch (gdbctx->in_packet[0]) { @@ -2008,11 +2082,16 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) return packet_ok; if (strncmp(gdbctx->in_packet, "Supported", 9) == 0) { + size_t i; + packet_reply_open(gdbctx); packet_reply_add(gdbctx, "QStartNoAckMode+;"); - packet_reply_add(gdbctx, "qXfer:libraries:read+;"); - packet_reply_add(gdbctx, "qXfer:threads:read+;"); - packet_reply_add(gdbctx, "qXfer:features:read+;"); + for (i = 0; i < ARRAY_SIZE(qxfer_handlers); i++) + { + packet_reply_add(gdbctx, "qXfer:"); + packet_reply_add(gdbctx, qxfer_handlers[i].name); + packet_reply_add(gdbctx, ":read+;"); + } packet_reply_close(gdbctx); return packet_done; } @@ -2043,53 +2122,47 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) } break; case 'X': - if (sscanf(gdbctx->in_packet, "Xfer:libraries:read::%x,%x", &off, &len) == 2) + if (parse_xfer_read(gdbctx->in_packet, object_name, annex, &off, &len)) { + enum packet_return result; + int i; BOOL more;
- if (!gdbctx->process) return packet_error; + for (i = 0; ; i++) + { + if (i >= ARRAY_SIZE(qxfer_handlers)) + { + ERR("unhandled qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len); + return packet_error; + } + if (strncmp(qxfer_handlers[i].name, object_name, QX_NAME_SIZE) == 0) + break; + }
- reply_buffer_empty(&gdbctx->qxfer_buffer); - packet_query_libraries(gdbctx); - packet_reply_xfer(gdbctx, - gdbctx->qxfer_buffer.base, - gdbctx->qxfer_buffer.len, - off, len, &more); - reply_buffer_empty(&gdbctx->qxfer_buffer); - return packet_done; - } - - if (sscanf(gdbctx->in_packet, "Xfer:threads:read::%x,%x", &off, &len) == 2) - { - BOOL more; - - if (!gdbctx->process) return packet_error; + TRACE("qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len);
reply_buffer_empty(&gdbctx->qxfer_buffer); - packet_query_threads(gdbctx); - packet_reply_xfer(gdbctx, - gdbctx->qxfer_buffer.base, - gdbctx->qxfer_buffer.len, - off, len, &more); - reply_buffer_empty(&gdbctx->qxfer_buffer); - return packet_done; - }
- if (sscanf(gdbctx->in_packet, "Xfer:features:read:target.xml:%x,%x", &off, &len) == 2) - { - BOOL more; + gdbctx->qxfer_object_idx = i; + lstrcpynA(gdbctx->qxfer_object_annex, annex, QX_ANNEX_SIZE);
- if (!gdbctx->process) return packet_error; - if (!(cpu = gdbctx->process->be_cpu)) return packet_error; + result = (*qxfer_handlers[i].handler)(gdbctx); + TRACE("qXfer read result = %d\n", result);
+ if ((result & ~packet_last_f) == packet_send_buffer) + { + packet_reply_xfer(gdbctx, + gdbctx->qxfer_buffer.base, + gdbctx->qxfer_buffer.len, + off, len, &more); + result = (result & packet_last_f) | packet_done; + } + + gdbctx->qxfer_object_idx = -1; + memset(gdbctx->qxfer_object_annex, 0, QX_ANNEX_SIZE); reply_buffer_empty(&gdbctx->qxfer_buffer); - packet_query_target_xml(gdbctx, cpu); - packet_reply_xfer(gdbctx, - gdbctx->qxfer_buffer.base, - gdbctx->qxfer_buffer.len, - off, len, &more); - reply_buffer_empty(&gdbctx->qxfer_buffer); - return packet_done; + + return result; } break; } @@ -2408,6 +2481,8 @@ static BOOL gdb_init_context(struct gdb_context* gdbctx, unsigned flags, unsigne for (i = 0; i < ARRAY_SIZE(gdbctx->wine_segs); i++) gdbctx->wine_segs[i] = 0;
+ gdbctx->qxfer_object_idx = -1; + memset(gdbctx->qxfer_object_annex, 0, sizeof(gdbctx->qxfer_object_annex)); memset(&gdbctx->qxfer_buffer, 0, sizeof(gdbctx->qxfer_buffer));
/* wait for first trap */