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

[Andrew Burgess <aburgess@redhat.com>]

Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org> writes:

> 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.

Thanks, pushed.

Andrew


>
>> ---
>>  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