Edgar E. Iglesias wrote:
> On Thu, May 15, 2008 at 09:11:29AM -0500, Jason Wessel wrote:
>   
>> +    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?
>   


Given that put_packet is what really needed the memory and already had
its own private global, I modified put_packet to accept a length and
contain the memtohex invocation.

>
>   
>> +
>> +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?
>   

I explicitly did not use "monitor set/show" because I figured these
commands would have been used by the qemu monitor, even though they
are presently not used today to my surprise.  After you asked the
question it occurred to me that the "else if" block we have now will
take care of the problem, so long as unique variables are used.

Attached is the new version.

Jason.