Re: [Qemu-devel] [PATCH v1 5/5] gdbstub: revert to previous set_reg behaviour

[Aleksandar Markovic <aleksandar.m.mail@gmail.com>]

On Jul 10, 2019 11:30 AM, "Alex Bennée" <alex.bennee@linaro.org> wrote:
>
>
> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
>
> > On Jul 5, 2019 6:08 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote:
> >>
> >> The refactoring of handle_set_reg missed the fact we previously had
> >> responded with an empty packet when we were not using XML based
> >> protocols. This broke the fallback behaviour for architectures that
> >> don't have registers defined in QEMU's gdb-xml directory.
> >>
> >> Revert to the previous behaviour and clean up the commentary for what
> >> is going on.
> >>
> >> Fixes: 62b3320bddd
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Cc: Jon Doron <arilou@gmail.com>
> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> ---
> >
> > Do you plan to integrate this patch in 4.1?
> >
> > Thanks, Aleksandar
>
> Yes - I'm putting together a PR today.
>

That's great, thanks!!

Aleksandar

> >
> >>  gdbstub.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/gdbstub.c b/gdbstub.c
> >> index ea3349d1aa..b6df7ee25a 100644
> >> --- a/gdbstub.c
> >> +++ b/gdbstub.c
> >> @@ -1669,12 +1669,23 @@ static void handle_remove_bp(GdbCmdContext
> > *gdb_ctx, void *user_ctx)
> >>      put_packet(gdb_ctx->s, "E22");
> >>  }
> >>
> >> +/*
> >> + * handle_set/get_reg
> >> + *
> >> + * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is
available.
> >> + * This works, but can be very slow. Anything new enough to understand
> >> + * XML also knows how to use this properly. However to use this we
> >> + * need to define a local XML file as well as be talking to a
> >> + * reasonably modern gdb. Responding with an empty packet will cause
> >> + * the remote gdb to fallback to older methods.
> >> + */
> >> +
> >>  static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
> >>  {
> >>      int reg_size;
> >>
> >>      if (!gdb_has_xml) {
> >> -        put_packet(gdb_ctx->s, "E00");
> >> +        put_packet(gdb_ctx->s, "");
> >>          return;
> >>      }
> >>
> >> @@ -1694,11 +1705,6 @@ static void handle_get_reg(GdbCmdContext
*gdb_ctx,
> > void *user_ctx)
> >>  {
> >>      int reg_size;
> >>
> >> -    /*
> >> -     * Older gdb are really dumb, and don't use 'g' if 'p' is
avaialable.
> >> -     * This works, but can be very slow.  Anything new enough to
> >> -     * understand XML also knows how to use this properly.
> >> -     */
> >>      if (!gdb_has_xml) {
> >>          put_packet(gdb_ctx->s, "");
> >>          return;
> >> --
> >> 2.20.1
> >>
> >>
>
>
> --
> Alex Bennée