This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Ping. Eli, Joel requests review of the doc parts of this patch. On Thu, Apr 17, 2008 at 2:25 PM, Joel Brobecker <brobecker@adacore.com> wrote: > > 2008-04-16 Doug Evans <dje@google.com> > > > > * NEWS: Mention new /m modifier for disassemble command. > > * cli/cli-cmds.c (print_disassembly): New function. > > (disassemble_current_function): New function > > (disassemble_command): Recognize /m modifier, print mixed > > > source+assembly. > > (init_cli_cmds): Update disassemble help text. > > The code part of your patch is good to go, with a couple of tiny > adjustments. Let's wait for Eli's feedback before checking in, since > there is some documentation being adjusted in the code too. > > > > + printf_filtered ("Dump of assembler code "); > > + if (name != NULL) > > + { > > + printf_filtered ("for function %s:\n", name); > > + } > > + else > > + { > > + printf_filtered ("from %s to %s:\n", paddress (low), paddress (high)); > > + } > > We don't need the curly braces in this case. Can you remove them? > > > > + if (*arg == '\0') > > + error (_("Missing modifier.")); > > + > > + while (*arg && ! isspace (*arg)) > > + { > > + switch (*arg++) > > + { > > + case 'm': > > > + mixed_source_and_assembly = 1; > > + break; > > + default: > > + error (_("Invalid disassembly modifier.")); > > + } > > + } > > + > > + while (isspace (*arg)) > > + ++arg; > > The formatting looks weird, because some of the lines (the "while" lines > for instance) are using spaces instead of tabs. Could you fix that to > use TABs, please? > > > @@ -1383,6 +1439,7 @@ With two args if one is empty it stands > > > c = add_com ("disassemble", class_vars, disassemble_command, _("\ > > Disassemble a specified section of memory.\n\ > > Default is the function surrounding the pc of the selected frame.\n\ > > +With a /m modifier source lines, if available, are included.\n\ > > I'd like to have Eli's feedback on this change. I would phrase > differently (the current form is missing a coma to make it > intelligible): > > With a /m modifier, source lines are included (if available). ---------- Forwarded message ---------- From: Doug Evans <dje@google.com> Date: Wed, Apr 16, 2008 at 3:25 PM Subject: Re: [RFA] mixed source+assembly from cli disassemble To: Joel Brobecker <brobecker@adacore.com>, Michael Snyder <msnyder@specifix.com>, Eli Zaretskii <eliz@gnu.org> Cc: gdb-patches@sourceware.org Here is a revised patch with all requested changed. I changed the modifier to /m. I know it was only a suggestion Michael, but it grew on me. Ok to check in?
Attachment:
gdb-080416-disass-3.patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |