Re: [PATCH 5/6] Disassembly unit test: memory error

[Luis Machado <lgustavo@codesourcery.com>]

On 01/16/2017 04:02 AM, Yao Qi wrote:
> This patch adds a unit test about memory error occurs on reading
> memory, and check MEMORY_ERROR exception is always thrown.
>
> v2:
>  - use null_stream,
>
> gdb:
>
> 2017-01-10  Yao Qi  <yao.qi@linaro.org>
>
> 	* disasm-selftests.c (gdb_disassembler_memory_error_test): New function.
> 	(_initialize_disasm_test): Register
> 	gdb_disassembler_memory_error_test.
> ---
>  gdb/disasm-selftests.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
> index 46a0a21..ad2b4eb 100644
> --- a/gdb/disasm-selftests.c
> +++ b/gdb/disasm-selftests.c
> @@ -163,6 +163,48 @@ gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch)
>    SELF_CHECK (di.print_insn (0) == len);
>  }
>
> +/* Test disassembly on memory error.  */
> +
> +static void
> +gdb_disassembler_memory_error_test (struct gdbarch *gdbarch)

Same comment as the other patch. Do we need to repeat the context of the 
test in the function name?

> +{
> +  class gdb_disassembler_test : public gdb_disassembler
> +  {
> +  public:
> +    gdb_disassembler_test (struct gdbarch *gdbarch)
> +      : gdb_disassembler (gdbarch, null_stream (),
> +			  gdb_disassembler_test::read_memory)
> +    {
> +    }
> +
> +    static int read_memory (bfd_vma memaddr, gdb_byte *myaddr,
> +			    unsigned int len,
> +			    struct disassemble_info *info)
> +    {
> +      /* Always get an error.  */

/* Always return an error.  */

Also, should we return an explicit error like TARGET_XFER_E_IO or 
MEMORY_ERROR?

> +      return -1;
> +    }
> +  };
> +
> +  gdb_disassembler_test di (gdbarch);
> +  bool see_memory_error = false;
> +
> +  TRY
> +    {
> +      di.print_insn (0);
> +    }
> +  CATCH (ex, RETURN_MASK_ERROR)
> +    {
> +      if (ex.error == MEMORY_ERROR)
> +	see_memory_error = true;
> +    }
> +  END_CATCH
> +
> +  /* Expect MEMORY_ERROR.   */

Too many spaces after period.

> +  SELF_CHECK (see_memory_error);
> +

Spurious newline?

> +}
> +
>  } // namespace selftests
>  #endif /* GDB_SELF_TEST */
>
> @@ -174,5 +216,6 @@ _initialize_disasm_test (void)
>  {
>  #if GDB_SELF_TEST
>    register_self_test (selftests::gdb_disassembler_print_one_insn_test);
> +  register_self_test (selftests::gdb_disassembler_memory_error_test);
>  #endif
>  }
>

Otherwise OK.