On Thu, Apr 09, 2020 at 07:51:49PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/9/20 6:49 PM, Peter Xu wrote:
> > This can expose the issue earlier on which register returned the wrong
> > result.
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > gdbstub.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 171e150950..f763545e81 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -911,17 +911,22 @@ static int gdb_read_register(CPUState *cpu,
> > GByteArray *buf, int reg)
> > CPUClass *cc = CPU_GET_CLASS(cpu);
> > CPUArchState *env = cpu->env_ptr;
> > GDBRegisterState *r;
> > + int len = 0, orig_len = buf->len;
> > if (reg < cc->gdb_num_core_regs) {
> > - return cc->gdb_read_register(cpu, buf, reg);
> > + len = cc->gdb_read_register(cpu, buf, reg);
>
> This change the code flow. We could add ...:
I didn't expect the "if" and "for" would collapse each other, but yeah
that could still be better.
Thanks,
>
> goto out;
>
> > }
>
> ... or use else?
> > for (r = cpu->gdb_regs; r; r = r->next) {
> > if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
> > - return r->get_reg(env, buf, reg - r->base_reg);
> > + len = r->get_reg(env, buf, reg - r->base_reg);
> > + break;
> > }
> > }
> > - return 0;
> > +
>
> out:
>
> > + assert(len == buf->len - orig_len);
> > +
> > + return len;
> > }
> > static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
> >
>
--
Peter Xu
