This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] gdb: Move common MI code to outer function.
- From: Doug Evans <xdje42 at gmail dot com>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 16 Sep 2015 19:54:14 -0700
- Subject: Re: [PATCH 1/2] gdb: Move common MI code to outer function.
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1440934890 dot git dot andrew dot burgess at embecosm dot com> <cover dot 1440934890 dot git dot andrew dot burgess at embecosm dot com> <dfbf766b7443265338ad1c6a87e774a7f58509e5 dot 1440934890 dot git dot andrew dot burgess at embecosm dot com>
Andrew Burgess <andrew.burgess@embecosm.com> writes:
> The disassembly code has a common entry function gdb_disassembly, and
> three handler functions which are called based on what type of
> disassembly we are performing.
>
> Each of the handler functions creates an MI list called asm_insns, which
> is required for the MI disassembly output.
>
> The commit moves the common asm_insns list to the outer level
> gdb_disassembly function, reducing duplicate code.
>
> gdb/ChangeLog:
>
> * disasm.c (do_mixed_source_and_assembly_deprecated): Remove
> asm_insns list.
> (do_mixed_source_and_assembly): Likewise.
> (do_assembly_only): Likewise.
> (gdb_disassembly): Create asm_insns list here.
Hi.
fwiw, I've got reservations about this cleanup.
Each of these do_* functions is tasked with emitting
the disassembly, and splitting out "asm_insns" makes
them no longer self-contained as far as what is emitted goes
(feels like a net loss in readability).
If someone else wants to approve this, go for it.
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index 2b65c6a..3032090 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -291,7 +291,6 @@ do_mixed_source_and_assembly_deprecated
> int next_line = 0;
> int num_displayed = 0;
> enum print_source_lines_flags psl_flags = 0;
> - struct cleanup *ui_out_chain;
> struct cleanup *ui_out_tuple_chain = make_cleanup (null_cleanup, 0);
> struct cleanup *ui_out_list_chain = make_cleanup (null_cleanup, 0);
>
btw, I think(!) the initial null_cleanups in
do_mixed_source_and_assembly_deprecated is wrong,
but it's been like that for awhile so maybe it's harmless.
I don't have time to dig deeper, just filing this as fyi.