Re: [PATCH] gdb/disasm: combine the no printing disassembler setup code

[Joel Brobecker <brobecker@adacore.com>]

Hi Andrew,

> We have three places in gdb where we initialise a disassembler that
> will not print anything (used for figuring out the length of
> instructions, or collecting other information from the disassembler).
> 
> Each of these places has its own stub function to act as a print like
> callback, the stub function is identical in each case, and just does
> nothing.
> 
> In this commit I create a new function to initialise a disassembler
> that doesn't print anything, and have all three locations use this new
> function.  There's now only one non-printing stub function.
> 
> There should be no user visible changes after this commit.

I took a look at the patch, and it looks good to me.

> ---
>  gdb/arc-tdep.c  | 10 ++--------
>  gdb/disasm.c    | 17 ++++++++++++-----
>  gdb/disasm.h    |  6 ++++++
>  gdb/s12z-tdep.c | 10 +++-------
>  4 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> index 90ec323d05e..297f83b8650 100644
> --- a/gdb/arc-tdep.c
> +++ b/gdb/arc-tdep.c
> @@ -1306,19 +1306,13 @@ arc_is_in_prologue (struct gdbarch *gdbarch, const struct arc_instruction &insn,
>    return false;
>  }
>  
> -/* Copy of gdb_buffered_insn_length_fprintf from disasm.c.  */
> -
> -static int ATTRIBUTE_PRINTF (2, 3)
> -arc_fprintf_disasm (void *stream, const char *format, ...)
> -{
> -  return 0;
> -}
> +/* See arc-tdep.h.  */
>  
>  struct disassemble_info
>  arc_disassemble_info (struct gdbarch *gdbarch)
>  {
>    struct disassemble_info di;
> -  init_disassemble_info (&di, &null_stream, arc_fprintf_disasm);
> +  init_disassemble_info_for_no_printing (&di);
>    di.arch = gdbarch_bfd_arch_info (gdbarch)->arch;
>    di.mach = gdbarch_bfd_arch_info (gdbarch)->mach;
>    di.endian = gdbarch_byte_order (gdbarch);
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index 5cd1f5adbd2..8f04fe6438d 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -891,16 +891,23 @@ gdb_insn_length (struct gdbarch *gdbarch, CORE_ADDR addr)
>    return gdb_print_insn (gdbarch, addr, &null_stream, NULL);
>  }
>  
> -/* fprintf-function for gdb_buffered_insn_length.  This function is a
> -   nop, we don't want to print anything, we just want to compute the
> -   length of the insn.  */
> +/* An fprintf-function for use by the disassembler when we know we don't
> +   want to print anything.  Always returns success.  */
>  
>  static int ATTRIBUTE_PRINTF (2, 3)
> -gdb_buffered_insn_length_fprintf (void *stream, const char *format, ...)
> +gdb_disasm_null_printf (void *stream, const char *format, ...)
>  {
>    return 0;
>  }
>  
> +/* See disasm.h.  */
> +
> +void
> +init_disassemble_info_for_no_printing (struct disassemble_info *dinfo)
> +{
> +  init_disassemble_info (dinfo, nullptr, gdb_disasm_null_printf);
> +}
> +
>  /* Initialize a struct disassemble_info for gdb_buffered_insn_length.
>     Upon return, *DISASSEMBLER_OPTIONS_HOLDER owns the string pointed
>     to by DI.DISASSEMBLER_OPTIONS.  */
> @@ -912,7 +919,7 @@ gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch,
>  				   CORE_ADDR addr,
>  				   std::string *disassembler_options_holder)
>  {
> -  init_disassemble_info (di, NULL, gdb_buffered_insn_length_fprintf);
> +  init_disassemble_info_for_no_printing (di);
>  
>    /* init_disassemble_info installs buffer_read_memory, etc.
>       so we don't need to do that here.
> diff --git a/gdb/disasm.h b/gdb/disasm.h
> index d739b57d898..359fb6a67fd 100644
> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -174,4 +174,10 @@ extern char *get_disassembler_options (struct gdbarch *gdbarch);
>  
>  extern void set_disassembler_options (const char *options);
>  
> +/* Setup DINFO with its output function and output stream setup so that
> +   nothing is printed while disassembling.  */
> +
> +extern void init_disassemble_info_for_no_printing
> +  (struct disassemble_info *dinfo);
> +
>  #endif
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index 3f9740f0dfe..659adf4f505 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -140,19 +140,15 @@ s12z_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int num)
>  
>  /* Support functions for frame handling.  */
>  
> -/* Copy of gdb_buffered_insn_length_fprintf from disasm.c.  */
>  
> -static int ATTRIBUTE_PRINTF (2, 3)
> -s12z_fprintf_disasm (void *stream, const char *format, ...)
> -{
> -  return 0;
> -}
> +/* Return a disassemble_info initialized for s12z disassembly, however,
> +   the disassembler will not actually print anything.  */
>  
>  static struct disassemble_info
>  s12z_disassemble_info (struct gdbarch *gdbarch)
>  {
>    struct disassemble_info di;
> -  init_disassemble_info (&di, &null_stream, s12z_fprintf_disasm);
> +  init_disassemble_info_for_no_printing (&di);
>    di.arch = gdbarch_bfd_arch_info (gdbarch)->arch;
>    di.mach = gdbarch_bfd_arch_info (gdbarch)->mach;
>    di.endian = gdbarch_byte_order (gdbarch);
> -- 
> 2.25.4
> 

-- 
Joel