Re: [PATCH] gdbstub/user-target: fix gdbserver int format (%d -> %x)

[Alex Bennée]

Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com> writes:

> From: disconnect3d <dominik.b.czarnota@gmail.com>

You might want to fix your author attribution here.

>
> This commit fixes an incorrect format string for formatting integers
> provided to GDB when debugging a target run in QEMU user mode.
>
> The correct format is hexadecimal for both success and errno values,
> some of which can be seen here [0].
>
> [0]
> https://github.com/bminor/binutils-gdb/blob/e65a355022d0dc6b5707310876a72b5693ec0aa5/gdbserver/hostio.cc#L196-L213

The spec isn't totally clear on this but it seems the match
remote_hostio_parse_result() as well:

  /* Check for ",errno".  */
  if (*p == ',')
    {
      errno = 0;
      *remote_errno = (fileio_error) strtol (p + 1, &p2, 16);
      if (errno != 0 || p + 1 == p2)
        return -1;
      p = p2;
    }

So: Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


You are also missing a s-o-b: 
https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#id29


> ---
>  gdbstub/user-target.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
> index 22bf4008c0..4bfcf78aaa 100644
> --- a/gdbstub/user-target.c
> +++ b/gdbstub/user-target.c
> @@ -317,9 +317,9 @@ void gdb_handle_v_file_open(GArray *params, void 
> *user_ctx)
>      int fd = open(filename, flags, mode);
>  #endif
>      if (fd < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>      } else {
> -        g_string_printf(gdbserver_state.str_buf, "F%d", fd);
> +        g_string_printf(gdbserver_state.str_buf, "F%x", fd);
>      }
>      gdb_put_strbuf();
>  }
> @@ -329,7 +329,7 @@ void gdb_handle_v_file_close(GArray *params, void 
> *user_ctx)
>      int fd = gdb_get_cmd_param(params, 0)->val_ul;
>  
>      if (close(fd) == -1) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -352,7 +352,7 @@ void gdb_handle_v_file_pread(GArray *params, void 
> *user_ctx)
>  
>      ssize_t n = pread(fd, buf, bufsiz, offset);
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }
> @@ -375,7 +375,7 @@ void gdb_handle_v_file_readlink(GArray *params, void 
> *user_ctx)
>      ssize_t n = readlink(filename, buf, BUFSIZ);
>  #endif
>      if (n < 0) {
> -        g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
> +        g_string_printf(gdbserver_state.str_buf, "F-1,%x", errno);
>          gdb_put_strbuf();
>          return;
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro