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: Sun, 04 Oct 2015 21:19:38 -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> <m337ydu561 dot fsf at sspiff dot org> <20150917100225 dot GI32100 at embecosm dot com>
Andrew Burgess <andrew.burgess@embecosm.com> writes:
> * Doug Evans <xdje42@gmail.com> [2015-09-16 19:54:14 -0700]:
>
>> 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).
>
> I think this patch change would be required if the second patch in the
> series is going to make it in.
>
> For now lets ignore the deprecated case.
>
> We basically have two disassemble methods, one is raw instructions,
> the other is instructions with source.
>
> In the pre-patch source the top level function looks up helper
> information symtab, etc then forwards the disassemble request to the
> correct helper.
>
> In the post-patch source the control flow keeps returning to the top
> level function each time it reaches the end of a symtab in order to
> discover the "next" symtab.
>
> Given that we want the complete disassembly to be nested under a
> single "asm_insns" that would need to be created at the top level (in
> gdb_disassembly)
Ok, fair enough.
At least this is all internal to disasm.c.