From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JwllH-0003jn-C5 for qemu-devel@nongnu.org; Thu, 15 May 2008 18:17:19 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JwllG-0003jb-VD for qemu-devel@nongnu.org; Thu, 15 May 2008 18:17:19 -0400 Received: from [199.232.76.173] (port=49993 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JwllG-0003jY-NS for qemu-devel@nongnu.org; Thu, 15 May 2008 18:17:18 -0400 Received: from miranda.se.axis.com ([193.13.178.8]:53697) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JwllG-0001iD-6f for qemu-devel@nongnu.org; Thu, 15 May 2008 18:17:18 -0400 Received: from axis.com (edgar.se.axis.com [10.93.151.1]) by miranda.se.axis.com (8.13.4/8.13.4/Debian-3sarge3) with ESMTP id m4FMHGaB014186 for ; Fri, 16 May 2008 00:17:16 +0200 Date: Fri, 16 May 2008 00:17:16 +0200 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support Message-ID: <20080515221716.GC27300@edgar.se.axis.com> References: <1210860693-22245-1-git-send-email-jason.wessel@windriver.com> <1210860693-22245-2-git-send-email-jason.wessel@windriver.com> <1210860693-22245-3-git-send-email-jason.wessel@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1210860693-22245-3-git-send-email-jason.wessel@windriver.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wessel Cc: qemu-devel@nongnu.org On Thu, May 15, 2008 at 09:11:30AM -0500, Jason Wessel wrote: > This patch adds a feature to the gdbstub to allow gdb to issue monitor > commands that can pass-through to the qemu monitor. > > In order to make this work, the MAX_MON (the maximum number of monitor > connections) had to get incremented by 1 to support the case when the > gdbstub is setup along with all the other connections. A small check > was added avoid strange crashes when you allocate too many qemu > monitor connections. > > The monitor_handle_command had to be exported in order to allow > gdbstub to pass the strings directly to the monitor. The gdbstub > registers as an output device of the qemu monitor such that no further > changes were needed to the monitor other than to have a global > variable in the gdbstub that controls when to transmit data from a > pass through monitor command to an attached debugger. > > The qemu docs were updated to reflect this change. > > Signed-off-by: Jason Wessel This sounds like a neat feature to me. AFAICS the patch looks mostly ok, just a few nitpicks. > --- > console.h | 1 + > gdbstub.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- > monitor.c | 8 ++++++-- > qemu-doc.texi | 11 +++++++++++ > 4 files changed, 61 insertions(+), 6 deletions(-) > > diff --git a/console.h b/console.h > index c7f29f5..041b4ef 100644 > --- a/console.h > +++ b/console.h > @@ -159,6 +159,7 @@ extern uint8_t _translate_keycode(const int key); > does not need to include console.h */ > /* monitor.c */ > void monitor_init(CharDriverState *hd, int show_banner); > +void monitor_handle_command(const char *cmdline); > void term_puts(const char *str); > void term_vprintf(const char *fmt, va_list ap); > void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2))); > diff --git a/gdbstub.c b/gdbstub.c > index 4bc7999..833cdd9 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -33,6 +33,7 @@ > #include "qemu-char.h" > #include "sysemu.h" > #include "gdbstub.h" > +#include "console.h" > #endif > > #include "qemu_socket.h" > @@ -70,6 +71,8 @@ typedef struct GDBState { > int running_state; > #else > CharDriverState *chr; > + CharDriverState *mon; > + int allow_monitor; > #endif > } GDBState; > > @@ -239,10 +242,11 @@ static int put_packet(GDBState *s, char *buf) > return 0; > } > > -static void monitor_output(GDBState *s, const char *msg) > +static void monitor_output_len(GDBState *s, const char *msg, int len) > { > char *buf; > - int len = strlen(msg); > + if (!len) > + len = strlen(msg); > > buf = malloc(len * 2 + 2); > buf[0] = 'O'; > @@ -251,6 +255,11 @@ static void monitor_output(GDBState *s, const char *msg) > free(buf); > } > > +static void monitor_output(GDBState *s, const char *msg) > +{ > + monitor_output_len(s, msg, 0); > +} > + If you change the zero argument into a strlen(msg) you can drop the check in monitor_output_len(). > static void monitor_help(GDBState *s) > { > monitor_output(s, "gdbstub specific monitor commands:\n"); > @@ -258,6 +267,7 @@ static void monitor_help(GDBState *s) > monitor_output(s, "set s_step <0|1> -- Single Stepping enabled\n"); > monitor_output(s, "set s_irq <0|1> -- Single Stepping with qemu irq handlers enabled\n"); > monitor_output(s, "set s_timer <0|1> -- Single Stepping with qemu timers enabled\n"); > + monitor_output(s, "qemu monitor pass-through commands:\n"); > } You might want to consider not showing that last line for CONFIG_USER_ONLY. Best regards -- Edgar E. Iglesias Axis Communications AB