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

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

[Andrew Burgess via Gdb-patches]

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

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

[Joel Brobecker via Gdb-patches]

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

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

[Andrew Burgess via Gdb-patches]

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