This is the mail archive of the gdb-patches@sources.redhat.com 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]

Re: [RFA/RFC] New command: ``start''


Eli,

Thanks for your useful comments. Attached are a couple of revised
patches based on your input.

> > The other decision I made was to allow this new method to be undefined
> > (NULL). In that case, we use "main" as the location where to insert the
> > temporary breakpoint. I am not too sure about this approach, but I
> > selected it because it avoids a xstrdup ("main")/xfree sequence.
> > On the other hand, forcing the method to always be set would make the
> > implemention clearer, I think, just at the expense of an unnecessary
> > xstrdup/xfree sequence.
> 
> I think it's better to have a non-NULL string there.  The benfit is
> that whoever reads the code doesn't need to look elsewhere to
> understand what effect does NULL have there.

OK. So I defined a new procedure in language.h, that returns a strdup
of "main". That way, all languages use the same approach, rather than
having only a few language having a non-null main_procedure_name
language method. Nice and consistent.

Just one additional comment, that may not be obvious: In Ada, the name
of the main procedure is not specified by Ada, it's up to the user to
choose it. That means that the name of that main procedure depends on
the program. When Ada-support is activated, we'll implement our own
main_procedure_name method that will dig out that name from the
executable. JIC.

> If we use "main" when the method is NULL, we should also use "main"
> if the method returns NULL, I think.

I changed the implementation to force the method to be defined.
I prefer that we report an error when the method failed to find the
name of the main procedure, rather than silently stop in start.

> > +  c = add_com ("start", class_run, start_command,
> > +               "\
> > +Start the debugged program until the beginning of the main procedure.\n\
> 
> I think this should say "Run the debugged program until ...".  "Start
> until" sounds like incorrect English.

Fixed. I also modified the description in an attempt to clarify how
arguments to this function are used.

> Please use "C@t{++}" instead of "C++", the former looks prettier in
> print.

Fixed.

> > +@code{main()}, but other languages such as Ada do not require a specific
> 
> Please say "@code{main}", without the parens.  "main()" looks like a
> call to `main' with no arguments, which is not what you mean here.

Fixed.

> > +The @code{start} command does the equivalent of setting a temporary
> > +breakpoint at the beginning of the main procedure and then performing
> > +a @code{run}.
> 
> `run' should be in @samp here, and I think "invoking the @samp{run}
> command" is better than "performing a @samp{run}".

Fixed. I also put the `start' in @samp, for consistency. I hope it was
the right thing to do.

> > Some programs contain an elaboration phase that will be
> > +performed before the main procedure is reached, and it is possible that
> > +the debugger will stop before reaching the main procedure. However,
> > +the temporary breakpoint will remain to halt execution.
> 
> Sorry, I don't understand this part (what is ``elaboration''?).
> Could you perhaps make this text more explanatory?

I tried to make it more explanatory by explaining what happens during
the elaboration (some code is run before the main procedure is called),
and also by providing an example using C++ constructors.

> > +The arguments used when using this command are directly passed to the
> > +@code{run} command.
> 
> You mean, if you type "run" thereafter, it will reuse the arguments
> you type for the "start" command?  If so, we should reword this text
> to make that more clear.  ``Directly passed'' is confusing, since
> nothing is really passed anywhere.

I have reworked the documentation. Is it better?
I was also wondering whether it would be better to switch para 1 and 2.
That way, the user immediately knows what this command does. He then
gets the explanation of what the "main procedure" is.

> Finally, our standard is to have 2 spaces after the period that ends a
> sentence.  Please make sure you do that in your patch.

Ouch! Sorry about that. I should know this by now, but I don't have
the reflex to remember it for the docs yet.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * language.h (language_defn): New field, la_xmain_procedure_name.
        (xdefault_main_program_name): Add declaration.
        * language.c (xdefault_main_program_name): New function.
        (unknown_language_defn): Set new field to xdefault_main_program_name.
        (auto_language_defn): Likewise.
        (local_language_defn): Likewise.
        * ada-lang.c (ada_language_defn): Likewise.
        * c-lang.c (c_language_defn): Likewise.
        (cplus_language_defn): Likewise.
        (asm_language_defn): Likewise.
        (minimal_language): Likewise.
        * f-lang.c (f_language_defn): Likewise.
        * jv-lang.c (java_language_defn): Likewise.
        * m2-lang.c (m2_language_defn): Likewise.
        * objc_language (objc_language_defn): Likewise.
        * p-lang.c (pascal_language_defn): Likewise.
        * scm-lang.c (scm_language_defn): Likewise.
        * infcmd.c (kill_if_already_running): New function, extracted
        from run_command().
        (run_command): Replace extracted code by call to
        kill_if_already_running().
        (start_command): New function.
        (_initialize_infcmd): Add "start" command.

2004-05-17  Joel Brobecker  <brobecker@gnat.com>

        * gdb.texinfo (Starting): Document new start command.

-- 
Joel

Attachment: start_cmd.diff
Description: Text document

Attachment: start_doc.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]