[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets |
Date: |
Thu, 15 May 2008 23:33:48 +0200 |
User-agent: |
Mutt/1.5.16 (2007-06-09) |
On Thu, May 15, 2008 at 09:11:29AM -0500, Jason Wessel wrote:
> At the gdbserial protocol level, using the qRcmd packets allows gdb to
> use the "monitor" command to access the controls for single stepping
> behavior. Now you can use a gdb "monitor" command instead of a gdb
> "maintenance" command.
>
> The qemu docs were updated to reflect this change.
>
> Signed-off-by: Jason Wessel <address@hidden>
Thanks Jason, a few comments inlined.
> ---
> gdbstub.c | 82
> +++++++++++++++++++++++++++++++++++++++++----------------
> qemu-doc.texi | 34 +++++++++++------------
> 2 files changed, 75 insertions(+), 41 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 9a361e3..4bc7999 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -239,6 +239,27 @@ static int put_packet(GDBState *s, char *buf)
> return 0;
> }
>
> +static void monitor_output(GDBState *s, const char *msg)
> +{
> + char *buf;
> + int len = strlen(msg);
> +
> + buf = malloc(len * 2 + 2);
> + buf[0] = 'O';
> + memtohex(buf + 1, msg, len);
> + put_packet(s, buf);
> + free(buf);
> +}
It feels odd that the code path that ends up here has line_buf, buf and mem_buf
available for parsing the query and generating the response and still we need
to malloc for more. Isn't there a way to reuse some of that memory?
> +
> +static void monitor_help(GDBState *s)
> +{
> + monitor_output(s, "gdbstub specific monitor commands:\n");
> + monitor_output(s, "s_show -- Show gdbstub Single Stepping variables\n");
> + 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");
> +}
I'd prefer if either have a show/set interface or we don't, this is kind of a
mix.
Some suggestions:
monitor sstep enable/disable
monitor sstep irq/noirq
monitor sstep timers/notimers
monitor sstep show
or:
monitor set/show sstep
monitor set/show sirq
monitor set/show stimers
What do you think?
Best regards
--
Edgar E. Iglesias
Axis Communications AB
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, (continued)
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Edgar E. Iglesias, 2008/05/15
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/19
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Paul Brook, 2008/05/19
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Maxim Gorbachyov, 2008/05/21
- Re: [Qemu-devel] [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/21
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/22
- Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/22
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/22
- Re: [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jason Wessel, 2008/05/22
- [Qemu-devel] Re: [PATCH 2/5] gdbstub: gdb pass-through qemu monitor support, Jan Kiszka, 2008/05/23
Re: [Qemu-devel] [PATCH 1/5] gdbstub: replace singlestep q packets with qRcmd packets,
Edgar E. Iglesias <=