Re: [PATCH] gdb: Fix riscv ARI issues

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Wednesday, March 07 2018, Andrew Burgess wrote:

> Fixes some ARI issues in recently added riscv code, the ARI email is:
>
>   https://sourceware.org/ml/gdb-patches/2018-03/msg00156.html

Thanks for the patch, Andrew.  Comment below.

> gdb/ChangeLog:
>
> 	* riscv-tdep.c (riscv_register_name): Use xsnprintf instead of
> 	sprintf.
> 	(riscv_insn::fetch_instruction): Use gdb_assert instead of
> 	internal_error.
> 	(riscv_print_arg_location): Use gdb_assert_not_reached instead of
> 	error.
> 	(riscv_push_dummy_call): Likewise.
> ---
>  gdb/ChangeLog    | 10 ++++++++++
>  gdb/riscv-tdep.c | 14 ++++++--------
>  2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 11b12279321..d84e7aba76a 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -481,7 +481,7 @@ riscv_register_name (struct gdbarch *gdbarch, int regnum)
>      {
>        static char buf[20];
>  
> -      sprintf (buf, "csr%d", regnum - RISCV_FIRST_CSR_REGNUM);
> +      xsnprintf (buf, 20, "csr%d", regnum - RISCV_FIRST_CSR_REGNUM);
>        return buf;
>      }
>  
> @@ -1049,12 +1049,10 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
>  
>    /* If we need more, grab it now.  */
>    instlen = riscv_insn_length (buf[0]);
> +  gdb_assert (instlen <= sizeof (buf));
>    *len = instlen;
> -  if (instlen > sizeof (buf))
> -    internal_error (__FILE__, __LINE__,
> -		    _("%s: riscv_insn_length returned %i"),
> -		    __func__, instlen);
> -  else if (instlen > 2)
> +
> +  if (instlen > 2)
>      {
>        status = target_read_memory (addr + 2, buf + 2, instlen - 2);
>        if (status)
> @@ -2009,7 +2007,7 @@ riscv_print_arg_location (ui_file *stream, struct gdbarch *gdbarch,
>        break;
>  
>      default:
> -      error ("unknown argument location type");
> +      gdb_assert_not_reached ("unknown argument location type");

Strings must be marked for localization, with _():

  gdb_assert_not_reached (_("unknown argument location type"));

I also think it's a good thing to start the sentence with a capital
letter.

>      }
>  }
>  
> @@ -2149,7 +2147,7 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
>  	  break;
>  
>  	default:
> -	  error ("unknown argument location type");
> +	  gdb_assert_not_reached ("unknown argument location type");

Likewise.

>  	}
>  
>        if (second_arg_length > 0)
> -- 
> 2.14.3

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/