Hi Alex, I did have some issues with the 'F' packet as it's not really
well documented, I suggest changing the schema to:
"L,L,o0"
so basically no support for anything after the first C in the Ctrl-C,
if you have a sample or a documentation that really implements
the F packet fully ill take a look at it and see how the schema should
really look like.
-- Jon.
On Wed, May 15, 2019 at 7:54 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Jon Doron <arilou@gmail.com> writes:
>
> There is a bit more going on here than a simple conversion. I think we
> need some additional commentary about the format of the data coming
> back.
>
>
> > Signed-off-by: Jon Doron <arilou@gmail.com>
> > ---
> > gdbstub.c | 62 +++++++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 40 insertions(+), 22 deletions(-)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 3478ac778d..9fe130f30d 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -1772,6 +1772,39 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
> > put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> > }
> >
> > +static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
> > +{
> > + int num_syscall_params;
> > + GdbCmdVariant syscall_params[3] = {};
> > +
> > + if (!gdb_ctx->num_params) {
> > + return;
> > + }
> > +
> > + if (cmd_parse_params(gdb_ctx->params[0].data, "L,L,o0", syscall_params,
> > + &num_syscall_params)) {
> > + return;
> > + }
>
> What's going on here? I thought the schema was meant to handle the
> parsing of data. I see bellow we originally parse the command as a null
> terminated string but we actually should handle:
>
> ‘Fretcode,errno,Ctrl-C flag;call-specific attachment’
>
> I see the argument for dealing with the call-specific attachment here
> but shouldn't the generic parsing code be able to split everything
> apart?
>
> > +
> > + if (!num_syscall_params) {
> > + return;
> > + }
> > +
> > + if (gdb_ctx->s->current_syscall_cb) {
> > + gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu,
> > + (target_ulong)syscall_params[0].val_ull,
> > + (target_ulong)syscall_params[1].val_ull);
> > + gdb_ctx->s->current_syscall_cb = NULL;
> > + }
>
>
>
> > +
> > + if (syscall_params[2].opcode == (uint8_t)'C') {
> > + put_packet(gdb_ctx->s, "T02");
> > + return;
> > + }
> > +
> > + gdb_continue(gdb_ctx->s);
> > +}
> > +
> > static int gdb_handle_packet(GDBState *s, const char *line_buf)
> > {
> > CPUState *cpu;
> > @@ -1913,28 +1946,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
> > return RS_IDLE;
> > case 'F':
> > {
> > - target_ulong ret;
> > - target_ulong err;
> > -
> > - ret = strtoull(p, (char **)&p, 16);
> > - if (*p == ',') {
> > - p++;
> > - err = strtoull(p, (char **)&p, 16);
> > - } else {
> > - err = 0;
> > - }
> > - if (*p == ',')
> > - p++;
> > - type = *p;
> > - if (s->current_syscall_cb) {
> > - s->current_syscall_cb(s->c_cpu, ret, err);
> > - s->current_syscall_cb = NULL;
> > - }
> > - if (type == 'C') {
> > - put_packet(s, "T02");
> > - } else {
> > - gdb_continue(s);
> > - }
> > + static const GdbCmdParseEntry file_io_cmd_desc = {
> > + .handler = handle_file_io,
> > + .cmd = "F",
> > + .cmd_startswith = 1,
> > + .schema = "s0"
> > + };
> > + cmd_parser = &file_io_cmd_desc;
> > }
> > break;
> > case 'g':
>
>
> --
> Alex Bennée