mbox series

[0/3] Minor cleanups in disassemble code

Message ID 20240321-disassemble-cleanup-v1-0-77448506f4ab@adacore.com
Headers
Series Minor cleanups in disassemble code |

Message

Tom Tromey March 21, 2024, 6:10 p.m. UTC 
  I happened to look at the disassemble code today, and I saw a few
minor changes that, IMO, make it a bit cleaner.

Regression tested on x86-64 Fedora 38.

---
Tom Tromey (3):
      Constify get_disassembler_options
      Remove some unnecessary casts
      Use std::string for disassembler options

 gdb/arc-tdep.c            | 15 +++++++--------
 gdb/arch-utils.c          |  6 +++---
 gdb/arm-tdep.c            |  6 +++---
 gdb/disasm.c              | 22 +++++++++-------------
 gdb/disasm.h              |  2 +-
 gdb/gdbarch-gen.h         |  4 ++--
 gdb/gdbarch.c             |  6 +++---
 gdb/gdbarch_components.py |  2 +-
 gdb/mips-tdep.c           | 14 +++++++-------
 gdb/riscv-tdep.c          |  2 +-
 gdb/rs6000-tdep.c         |  2 +-
 gdb/s390-tdep.c           |  2 +-
 12 files changed, 39 insertions(+), 44 deletions(-)
---
base-commit: acaf48b921453c37fc2df4151699c912940bcd25
change-id: 20240321-disassemble-cleanup-5da01a245e74

Best regards,

Comments

John Baldwin March 22, 2024, 5:59 p.m. UTC | #1 
On 3/21/24 11:10 AM, Tom Tromey wrote:
> I happened to look at the disassemble code today, and I saw a few
> minor changes that, IMO, make it a bit cleaner.
> 
> Regression tested on x86-64 Fedora 38.
> 
> ---
> Tom Tromey (3):
>        Constify get_disassembler_options
>        Remove some unnecessary casts
>        Use std::string for disassembler options
> 
>   gdb/arc-tdep.c            | 15 +++++++--------
>   gdb/arch-utils.c          |  6 +++---
>   gdb/arm-tdep.c            |  6 +++---
>   gdb/disasm.c              | 22 +++++++++-------------
>   gdb/disasm.h              |  2 +-
>   gdb/gdbarch-gen.h         |  4 ++--
>   gdb/gdbarch.c             |  6 +++---
>   gdb/gdbarch_components.py |  2 +-
>   gdb/mips-tdep.c           | 14 +++++++-------
>   gdb/riscv-tdep.c          |  2 +-
>   gdb/rs6000-tdep.c         |  2 +-
>   gdb/s390-tdep.c           |  2 +-
>   12 files changed, 39 insertions(+), 44 deletions(-)
> ---
> base-commit: acaf48b921453c37fc2df4151699c912940bcd25
> change-id: 20240321-disassemble-cleanup-5da01a245e74
> 
> Best regards,

These all seem sensible.  It seems a bit odd to me that
gdbarch_disassembler_options returns a pointer to the string rather
than the string, but that was true before and not related to your
changes.

Approved-By: John Baldwin <jhb@FreeBSD.org>
Tom Tromey March 22, 2024, 7:12 p.m. UTC | #2 
>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John> These all seem sensible.  It seems a bit odd to me that
John> gdbarch_disassembler_options returns a pointer to the string rather
John> than the string, but that was true before and not related to your
John> changes.

I guess it's so it can be set and fetched with a single api, and so
there can be a default of not letting the user touch the options for an
arch.  It is a little weird though now that you mention it.  Like why
have globals.  Cleanup for another day perhaps.

Tom