Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [v6] multi-executable support
@ 2009-10-05 15:59 Pedro Alves
  2009-10-05 21:57 ` Eli Zaretskii
  2009-10-06  5:13 ` [v6] " Sérgio Durigan Júnior
  0 siblings, 2 replies; 15+ messages in thread
From: Pedro Alves @ 2009-10-05 15:59 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

Here's my current patch to add multi-executable support to GDB.
Following the last discussion, 

 http://sourceware.org/ml/gdb-patches/2009-09/msg00194.html

This makes it so that there's always an inferior, and the
"symbol-spaces" are relegated to the background as an implementation
detail, renamed as program spaces.

This is almost the same as the patch I had posted on the link
above as "v5", with s/symbol-spaces/program-spaces/, generally
cleaned up, and documentation updated to reflect the simpler
model and to address other points Eli raised previously.

I'm hoping this isn't far from putting in, as it's been
a nuisance getting this patch close to mainline, as it
tends to regress easily due to new code going in that
doesn't consider multi-exec.

Eli, could you please take a new look at the docs bits?
I may have missed something, but I hope nothing big.

Note: the changeLogs are a bit stale and needs a
new update.

No regressions on x86_64-unknown-linux-gnu.

-- 
Pedro Alves

[-- Attachment #2: multiexec_v6.diff.gz --]
[-- Type: application/x-gzip, Size: 80507 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v6] multi-executable support
  2009-10-05 15:59 [v6] multi-executable support Pedro Alves
@ 2009-10-05 21:57 ` Eli Zaretskii
  2009-10-06 12:14   ` Pedro Alves
  2009-10-06  5:13 ` [v6] " Sérgio Durigan Júnior
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2009-10-05 21:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Mon, 5 Oct 2009 16:59:20 +0100
> 
> Eli, could you please take a new look at the docs bits?
> I may have missed something, but I hope nothing big.

Ack.  I also spotted a few typos in the comments, see below.

> +/* These functions concerns about actual breakpoints inserted in the

"These functions concerns" doesn't sound right.

> +/* Returns true if {ASPACE1,ADDR1} and {ASPACE2,ADDR2} represent the
> +   same breakpoint location.  In most targets, this is will be true if
                                                       ^^
I think "is" is redundant here.

> +    /* The program had previously vforked, and now the child is done
> +       with the shared memory region, because it exec'ed or exited.
> +       Note that the event if reported to the vfork parent.  This is
                              ^^
Did you mean "is"?

> +	  /* If this is a vfork child exiting, then the pspace and
> +	     aspaces where shared with the parent.  Since we're
                     ^^^^^
"were", right?

> Index: src/gdb/doc/gdb.texinfo

This is okay, but I have a few comments:

> +In addition, below each inferior line, @value{GDBN} prints extra
> +information that isn't suitable to display in tabular form.  For
> +example, extra vfork parent/child relationships.

How about showing some of that in the example?  Otherwise, this note
sounds too mysterious, IMO.

> +Many commands will work the same with multiple programs as with a
> +single program: E.g., @code{print myglobal} will simply display the
                   ^^^^
"e.g.", in lower case.

> +Ocasionaly, when debugging @value{GDBN} itself, it may be useful to
   ^^^^^^^^^^
"Occasionally"

> +get more info about the relationship or inferiors, programs, address
                                        ^^
"of", I presume

> +@smallexample
> +(@value{GDBP}) maint info program-spaces
> +  Id   Executable
> +  2    goodbye
> +        Bound inferiors: ID 1 (process 21561)
> +* 1    hello
> +@end smallexample
> +@end table
> +
> +Here we can see that no inferior is running the program @code{hello},
> +while @code{process 21561} is running the program @code{goodbye}.  On
> +some targets, it is possible that multiple inferiors are bound to the
> +same program space.  The most common example is that of debugging both
> +the parent and child processes of a @code{vfork}
> +call. @xref{Forks,,Debugging Forks}.

Since this is about program spaces, not about inferiors, I'd suggest
to show an example that includes multiple inferiors in the same
program space.  Otherwise, this looks awfully similar to "info
inferiors" and the purpose of this command remains unclear.

> +@code{follow-exec-mode} can be:
> +
> +
> +  new - the debugger creates a new inferior and rebinds the process \n\
> +to this new inferior.  The program the process was running before\n\
> +the exec call can be restarted afterwards by restarting the original\n\
> +inferior.\n\
> +\n\
> +  same - the debugger keeps the process bound to the same inferior.\n\
> +The new executable image replaces the previous executable loaded in\n\
> +the inferior.  Restarting the inferior after the exec call restarts\n\
> +the executable the process was running after the exec call.\n\
> +\n\
> +By default, the debugger will use the same inferior."),

This looks like copy/paste from the doc strings in the source.  It
should be removed, I think.

> +@value{GDBN} creates a new inferior and rebinds the process to this
> +new inferior.  The program the process was running before the exec
                                                                 ^^^^
"exec" should be in @code.

> +inferior.  Restarting the inferior after the exec call restarts the
> +executable the process was running after the exec call.  This is the
> +default mode.

Same here.

> +(@value{GDBP}) info sspaces

Should be "maint info program-spaces", right?

> Index: src/gdb/NEWS

This is okay, with a couple of comments:

> +maint info program-spaces
> +  List the program spaces loaded into GDB.

This should probably be after the *-inferior commands, not before them.

> +  When the program execs, request to keep the previous executable'
                                                          ^^^^^^^^^^^
"executable's", right?

> +  symbol loaded, and load the new executable in a new symbol-space; or
> +  request GDB to reuse the symbol-space and replace the previous
> +  executable's symbols with the new executable.

I think you want to replace "symbol space" with "program space".

> +/* Add a new empty program space to the program space list, and binds it
> +   to ASPACE.  Returns the pointer to the new object.  */

Either "add", "bind", and "return", or "adds", "binds", and "returns".

> +/* If ARGS is NULL or empty, print information about all symbol
> +   spaces.  Otherwise, ARGS is a text representation of a LONG

"program spaces".

> +/* All known objfiles are kept in a linked list.  This points to the
> +   root of this list. */

I believe "root" is used for trees.  Lists have a "head".


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v6] multi-executable support
  2009-10-05 15:59 [v6] multi-executable support Pedro Alves
  2009-10-05 21:57 ` Eli Zaretskii
@ 2009-10-06  5:13 ` Sérgio Durigan Júnior
  2009-10-06 13:55   ` Pedro Alves
  1 sibling, 1 reply; 15+ messages in thread
From: Sérgio Durigan Júnior @ 2009-10-06  5:13 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves

Hello Pedro,

On Monday 05 October 2009, Pedro Alves wrote:
> Here's my current patch to add multi-executable support to GDB.

Thanks for this nice effort.  I am trying to apply your patch using CVS HEAD, 
but there are two files that your patch tries to modify but don't exist on the 
repo:

src/gdb/testsuite/gdb.multi/base.exp, and
src/gdb/testsuite/gdb.multi/bkpt-multi-exec.exp

I may be doing something stupid here, but I'd like to double-check.

> I'm hoping this isn't far from putting in, as it's been
> a nuisance getting this patch close to mainline, as it
> tends to regress easily due to new code going in that
> doesn't consider multi-exec.

I know the feeling :-(.

Thanks again,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v6] multi-executable support
  2009-10-05 21:57 ` Eli Zaretskii
@ 2009-10-06 12:14   ` Pedro Alves
  2009-10-06 14:29     ` Eli Zaretskii
  2009-10-12  1:26     ` [v7] " Pedro Alves
  0 siblings, 2 replies; 15+ messages in thread
From: Pedro Alves @ 2009-10-06 12:14 UTC (permalink / raw)
  To: gdb-patches

On Monday 05 October 2009 22:55:09, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Mon, 5 Oct 2009 16:59:20 +0100
> > 
> > Eli, could you please take a new look at the docs bits?
> > I may have missed something, but I hope nothing big.
> 
> Ack.  I also spotted a few typos in the comments, see below.

Thanks.

> 
> > +/* These functions concerns about actual breakpoints inserted in the
> 
> "These functions concerns" doesn't sound right.

Fixed.

> 
> > +/* Returns true if {ASPACE1,ADDR1} and {ASPACE2,ADDR2} represent the
> > +   same breakpoint location.  In most targets, this is will be true if
>                                                        ^^
> I think "is" is redundant here.

Fixed.

> 
> > +    /* The program had previously vforked, and now the child is done
> > +       with the shared memory region, because it exec'ed or exited.
> > +       Note that the event if reported to the vfork parent.  This is
>                               ^^
> Did you mean "is"?

Yes.

> 
> > +	  /* If this is a vfork child exiting, then the pspace and
> > +	     aspaces where shared with the parent.  Since we're
>                      ^^^^^
> "were", right?

Right.

> 
> > Index: src/gdb/doc/gdb.texinfo
> 
> This is okay, but I have a few comments:
> 
> > +In addition, below each inferior line, @value{GDBN} prints extra
> > +information that isn't suitable to display in tabular form.  For
> > +example, extra vfork parent/child relationships.
> 
> How about showing some of that in the example?  Otherwise, this note
> sounds too mysterious, IMO.

The idea was to not distract the user here much with a use case
that isn't that common, and have she follow the xref
to see an example of that.  It seems I posted the example in the "Forks"
section, but didn't finish what I wanted to with it :-/  Will fix.

> 
> > +Many commands will work the same with multiple programs as with a
> > +single program: E.g., @code{print myglobal} will simply display the
>                    ^^^^
> "e.g.", in lower case.

Fixed.

> 
> > +Ocasionaly, when debugging @value{GDBN} itself, it may be useful to
>    ^^^^^^^^^^
> "Occasionally"
> 
> > +get more info about the relationship or inferiors, programs, address
>                                         ^^
> "of", I presume

Fixed and yes.

> 
> > +@smallexample
> > +(@value{GDBP}) maint info program-spaces
> > +  Id   Executable
> > +  2    goodbye
> > +        Bound inferiors: ID 1 (process 21561)
> > +* 1    hello
> > +@end smallexample
> > +@end table
> > +
> > +Here we can see that no inferior is running the program @code{hello},
> > +while @code{process 21561} is running the program @code{goodbye}.  On
> > +some targets, it is possible that multiple inferiors are bound to the
> > +same program space.  The most common example is that of debugging both
> > +the parent and child processes of a @code{vfork}
> > +call. @xref{Forks,,Debugging Forks}.
> 
> Since this is about program spaces, not about inferiors, I'd suggest
> to show an example that includes multiple inferiors in the same
> program space.  Otherwise, this looks awfully similar to "info
> inferiors" and the purpose of this command remains unclear.

Good idea, will do that.

> 
> > +@code{follow-exec-mode} can be:
> > +
> > +
> > +  new - the debugger creates a new inferior and rebinds the process \n\
> > +to this new inferior.  The program the process was running before\n\
> > +the exec call can be restarted afterwards by restarting the original\n\
> > +inferior.\n\
> > +\n\
> > +  same - the debugger keeps the process bound to the same inferior.\n\
> > +The new executable image replaces the previous executable loaded in\n\
> > +the inferior.  Restarting the inferior after the exec call restarts\n\
> > +the executable the process was running after the exec call.\n\
> > +\n\
> > +By default, the debugger will use the same inferior."),
> 
> This looks like copy/paste from the doc strings in the source.  It
> should be removed, I think.

Bah, of course.

> 
> > +@value{GDBN} creates a new inferior and rebinds the process to this
> > +new inferior.  The program the process was running before the exec
>                                                                  ^^^^
> "exec" should be in @code.
> 

Fixed.

> > +inferior.  Restarting the inferior after the exec call restarts the
> > +executable the process was running after the exec call.  This is the
> > +default mode.
> 
> Same here.
> 
> > +(@value{GDBP}) info sspaces
> 
> Should be "maint info program-spaces", right?

That would be the direct mapping, but "info inferiors" would
be better.  I add adjusted the "new" variant but missed
the "same" variant.  Thanks for catching.

> 
> > Index: src/gdb/NEWS
> 
> This is okay, with a couple of comments:
> 
> > +maint info program-spaces
> > +  List the program spaces loaded into GDB.
> 
> This should probably be after the *-inferior commands, not before them.

Hmm, bah, we missed something important here.  Since this feature missed
7.0, I get to add a "Changes since GDB 7.0" section, and rewire these
entries there.

> 
> > +  When the program execs, request to keep the previous executable'
>                                                           ^^^^^^^^^^^
> "executable's", right?

Right.

> 
> > +  symbol loaded, and load the new executable in a new symbol-space; or
> > +  request GDB to reuse the symbol-space and replace the previous
> > +  executable's symbols with the new executable.
> 
> I think you want to replace "symbol space" with "program space".

Yeah.  I think I should rewrite that entry a bit.  This text
predates the "always-an-inferior" change and the corresponding
docs/help entry of the setting doesn't describe the exact same
thing.

> 
> > +/* Add a new empty program space to the program space list, and binds it
> > +   to ASPACE.  Returns the pointer to the new object.  */
> 
> Either "add", "bind", and "return", or "adds", "binds", and "returns".

Fixed.

> 
> > +/* If ARGS is NULL or empty, print information about all symbol
> > +   spaces.  Otherwise, ARGS is a text representation of a LONG
> 
> "program spaces".

Clearly my grepping failed to consider lines breaks.  Fixed.

> > +/* All known objfiles are kept in a linked list.  This points to the
> > +   root of this list. */
> 
> I believe "root" is used for trees.  Lists have a "head".

I'm sure I could plant some species of lists.  :-P  Thanks, fixed.

I'll post a new patch later, and I'll re-quote the bits that need
review in particular.  Thanks much, I couldn't spot these issues even
after staring at the patch for long.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v6] multi-executable support
  2009-10-06  5:13 ` [v6] " Sérgio Durigan Júnior
@ 2009-10-06 13:55   ` Pedro Alves
  0 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2009-10-06 13:55 UTC (permalink / raw)
  To: Sérgio Durigan Júnior; +Cc: gdb-patches

On Tuesday 06 October 2009 06:13:11, Sérgio Durigan Júnior wrote:
> Hello Pedro,
> 
> On Monday 05 October 2009, Pedro Alves wrote:
> > Here's my current patch to add multi-executable support to GDB.
> 
> Thanks for this nice effort.  I am trying to apply your patch using CVS HEAD, 
> but there are two files that your patch tries to modify but don't exist on the 
> repo:
> 
> src/gdb/testsuite/gdb.multi/base.exp, and
> src/gdb/testsuite/gdb.multi/bkpt-multi-exec.exp
> 
> I may be doing something stupid here, but I'd like to double-check.

Nah, my fault.  I mis-flattened my patch set.  I'll post a new
fixed patch soon.  Thanks!

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v6] multi-executable support
  2009-10-06 12:14   ` Pedro Alves
@ 2009-10-06 14:29     ` Eli Zaretskii
  2009-10-12  1:26     ` [v7] " Pedro Alves
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2009-10-06 14:29 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 6 Oct 2009 13:14:29 +0100
> 
> I couldn't spot these issues even after staring at the patch for
> long.

There's nothing like another pair of eyes for that ;-)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [v7] multi-executable support
  2009-10-06 12:14   ` Pedro Alves
  2009-10-06 14:29     ` Eli Zaretskii
@ 2009-10-12  1:26     ` Pedro Alves
  2009-10-12 19:09       ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2009-10-12  1:26 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii

[-- Attachment #1: Type: text/plain, Size: 26724 bytes --]

Here's the updated patch with docs fixed per review.  Could
you please take another look?

On Tuesday 06 October 2009 13:14:29, Pedro Alves wrote:
> On Monday 05 October 2009 22:55:09, Eli Zaretskii wrote:
> > > From: Pedro Alves <pedro@codesourcery.com>

> > > +In addition, below each inferior line, @value{GDBN} prints extra
> > > +information that isn't suitable to display in tabular form.  For
> > > +example, extra vfork parent/child relationships.
> > 
> > How about showing some of that in the example?  Otherwise, this note
> > sounds too mysterious, IMO.
> 
> The idea was to not distract the user here much with a use case
> that isn't that common, and have she follow the xref
> to see an example of that.  It seems I posted the example in the "Forks"
> section, but didn't finish what I wanted to with it :-/  Will fix.

I ended up simply dropping this here.

> > > +@smallexample
> > > +(@value{GDBP}) maint info program-spaces
> > > +  Id   Executable
> > > +  2    goodbye
> > > +        Bound inferiors: ID 1 (process 21561)
> > > +* 1    hello
> > > +@end smallexample
> > > +@end table
> > > +
> > > +Here we can see that no inferior is running the program @code{hello},
> > > +while @code{process 21561} is running the program @code{goodbye}.  On
> > > +some targets, it is possible that multiple inferiors are bound to the
> > > +same program space.  The most common example is that of debugging both
> > > +the parent and child processes of a @code{vfork}
> > > +call. @xref{Forks,,Debugging Forks}.
> > 
> > Since this is about program spaces, not about inferiors, I'd suggest
> > to show an example that includes multiple inferiors in the same
> > program space.  Otherwise, this looks awfully similar to "info
> > inferiors" and the purpose of this command remains unclear.
> 
> Good idea, will do that.

I've added such example now.

> > > +inferior.  Restarting the inferior after the exec call restarts the
> > > +executable the process was running after the exec call.  This is the
> > > +default mode.
> > 
> > Same here.
> > 
> > > +(@value{GDBP}) info sspaces
> > 
> > Should be "maint info program-spaces", right?
> 
> That would be the direct mapping, but "info inferiors" would
> be better.  I add adjusted the "new" variant but missed
> the "same" variant.  Thanks for catching.

I've also redone this bit.

> > > Index: src/gdb/NEWS
> > 
> > This is okay, with a couple of comments:
> > 
> > > +maint info program-spaces
> > > +  List the program spaces loaded into GDB.
> > 
> > This should probably be after the *-inferior commands, not before them.
> 
> Hmm, bah, we missed something important here.  Since this feature missed
> 7.0, I get to add a "Changes since GDB 7.0" section, and rewire these
> entries there.

I've adjusted the NEWS entry.  Since I'm not readjusting the existing
multi-inferiors note anymore, this one looks different and needs
new review.  Sorry for the extra work.

> > > +  symbol loaded, and load the new executable in a new symbol-space; or
> > > +  request GDB to reuse the symbol-space and replace the previous
> > > +  executable's symbols with the new executable.
> > 
> > I think you want to replace "symbol space" with "program space".
> 
> Yeah.  I think I should rewrite that entry a bit.  This text
> predates the "always-an-inferior" change and the corresponding
> docs/help entry of the setting doesn't describe the exact same
> thing.

I've done this now.

> > > +/* If ARGS is NULL or empty, print information about all symbol
> > > +   spaces.  Otherwise, ARGS is a text representation of a LONG
> > 
> > "program spaces".
> 
> Clearly my grepping failed to consider lines breaks.  Fixed.

I've also fixed a bunch more such missing s/symbol/program/ cases.

> I'll post a new patch later, and I'll re-quote the bits that need
> review in particular.

There were no code changes in this revision.

I'm attaching the new updated patch (gzipped).  Here's the updated
changelog entry.

2009-10-12  Pedro Alves  <pedro@codesourcery.com>
            Stan Shebs  <stan@codesourcery.com>

        Add multi-executable/process support to GDB.

        gdb/
        * Makefile.in (SFILES): Add progspace.c.
        (COMMON_OBS): Add progspace.o.
        * progspace.h: New.
        * progspace.c: New.

        * breakpoint.h (struct bp_target_info) <placed_address_space>: New
        field.
        (struct bp_location) <pspace>: New field.
        (struct breakpoint) <pspace>: New field.
        (bpstat_stop_status, breakpoint_here_p)
        (moribund_breakpoint_here_p, breakpoint_inserted_here_p)
        (regular_breakpoint_inserted_here_p)
        (software_breakpoint_inserted_here_p, breakpoint_thread_match)
        (set_default_breakpoint): Adjust prototypes.
        (remove_breakpoints_pid, breakpoint_program_space_exit): Declare.
        (insert_single_step_breakpoint, deprecated_insert_raw_breakpoint):
        Adjust prototypes.
        * breakpoint.c (executing_startup): Delete.
        (default_breakpoint_sspace): New.
        (breakpoint_restore_shadows): Skip if the address space doesn't
        match.
        (update_watchpoint): Record the frame's program space in the
        breakpoint location.
        (insert_bp_location): Record the address space in target_info.
        Adjust to pass the symbol space to solib_name_from_address.
        (breakpoint_program_space_exit): New.
        (insert_breakpoint_locations): Switch the symbol space and thread
        when inserting breakpoints.  Don't insert breakpoints in a vfork
        parent waiting for vfork done if we're not attached to the vfork
        child.
        (remove_breakpoints_pid): New.
        (reattach_breakpoints): Switch to a thread of PID.  Ignore
        breakpoints of other symbol spaces.
        (create_internal_breakpoint): Store the symbol space in the sal.
        (create_longjmp_master_breakpoint): Iterate over all symbol
        spaces.
        (update_breakpoints_after_exec): Ignore breakpoints for other
        symbol spaces.
        (remove_breakpoint): Rename to ...
        (remove_breakpoint_1): ... this.  Pass the breakpoints symbol
        space to solib_name_from_address.
        (remove_breakpoint): New.
        (mark_breakpoints_out): Ignore breakpoints from other symbol
        spaces.
        (breakpoint_init_inferior): Ditto.
        (breakpoint_here_p): Add an address space argument and adjust to
        use breakpoint_address_match.
        (moribund_breakpoint_here_p): Ditto.
        (regular_breakpoint_inserted_here_p): Ditto.
        (breakpoint_inserted_here_p): Ditto.
        (software_breakpoint_inserted_here_p): Ditto.
        (breakpoint_thread_match): Ditto.
        (bpstat_check_location): Ditto.
        (bpstat_stop_status): Ditto.
        (print_breakpoint_location): If there's a location to print,
        switch the current symbol space.
        (print_one_breakpoint_location): Add `allflag' argument.
        (print_one_breakpoint): Ditto.  Adjust.
        (do_captured_breakpoint_query): Adjust.
        (breakpoint_1): Adjust.
        (breakpoint_has_pc): Also match the symbol space.
        (describe_other_breakpoints): Add a symbol space argument and
        adjust.
        (set_default_breakpoint): Add a symbol space argument.  Set
        default_breakpoint_sspace.
        (breakpoint_address_match): New.
        (check_duplicates_for): Add an address space argument, and adjust.
        (set_raw_breakpoint): Record the symbol space in the location and
        in the breakpoint.
        (set_longjmp_breakpoint): Skip longjmp master breakpoints from
        other symbol spaces.
        (remove_thread_event_breakpoints, remove_solib_event_breakpoints)
        (disable_breakpoints_in_shlibs): Skip breakpoints from other
        symbol spaces.
        (disable_breakpoints_in_unloaded_shlib): Match symbol spaces.
        (create_catchpoint): Set the symbol space in the sal.
        (disable_breakpoints_before_startup): Skip breakpoints from other
        symbol spaces.  Set executing_startup in the current symbol space.
        (enable_breakpoints_after_startup): Clear executing_startup in the
        current symbol space.  Skip breakpoints from other symbol spaces.
        (clone_momentary_breakpoint): Also copy the symbol space.
        (add_location_to_breakpoint): Set the location's symbol space.
        (bp_loc_is_permanent): Switch thread and symbol space.
        (create_breakpoint): Adjust.
        (expand_line_sal_maybe): Expand comment to mention symbol spaces.
        Switch thread and symbol space when reading memory.
        (parse_breakpoint_sals): Set the symbol space in the sal.
        (break_command_really): Ditto.
        (skip_prologue_sal): Switch and space.
        (resolve_sal_pc): Ditto.
        (watch_command_1): Record the symbol space in the sal.
        (create_ada_exception_breakpoint): Adjust.
        (clear_command): Adjust.  Match symbol spaces.
        (update_global_location_list): Use breakpoint_address_match.
        (breakpoint_re_set_one): Switch thread and space.
        (breakpoint_re_set): Save symbol space.
        (breakpoint_re_set_thread): Also reset the symbol space.
        (deprecated_insert_raw_breakpoint): Add an address space argument.
        Adjust.
        (insert_single_step_breakpoint): Ditto.
        (single_step_breakpoint_inserted_here_p): Ditto.
	(clear_syscall_counts): New.
	(_initialize_breakpoint): Install it as inferior_exit observer.

        * exec.h: Include "progspace.h".
        (exec_bfd, exec_bfd_mtime): New defines.
        (exec_close): Declare.
        * exec.c: Include "gdbthread.h" and "progspace.h".
        (exec_bfd, exec_bfd_mtime, current_target_sections_1): Delete.
        (using_exec_ops): New.
        (exec_close_1): Rename to exec_close, and make public.
        (exec_close): Rename to exec_close_1, and adjust all callers.  Add
	description.  Remove target sections and close executables from
	all program spaces.
        (exec_file_attach): Add comment.
        (add_target_sections): Check on `using_exec_ops' to check if the
        target should be pushed.
        (remove_target_sections): Only unpush the target if there are no
        more target sections in any symbol space.
        * gdbcore.h: Include "exec.h".
        (exec_bfd, exec_bfd_mtime): Remove declarations.

        * frame.h (get_frame_program_space, get_frame_address_space)
        (frame_unwind_program_space): Declare.
        * frame.c (struct frame_info) <pspace, aspace>: New fields.
        (create_sentinel_frame): Add program space argument.  Set the
        pspace and aspace fields of the frame object.
        (get_current_frame, create_new_frame): Adjust.
        (get_frame_program_space): New.
        (frame_unwind_program_space): New.
        (get_frame_address_space): New.
        * stack.c (print_frame_info): Adjust.
        (print_frame): Use the frame's program space.

        * gdbthread.h (any_live_thread_of_process): Declare.
        * thread.c (any_live_thread_of_process): New.
        (switch_to_thread): Switch the program space as well.
        (restore_selected_frame): Don't warn if trying to restore frame
        level 0.

        * inferior.h: Include "progspace.h".
        (detach_fork): Declare.
	
        (struct inferior) <removable, aspace, pspace>
	<vfork_parent, vfork_child, pending_detach>
	<waiting_for_vfork_done>: New fields.
        <terminal_info>: Remove field.
	<data, num_data>: New fields.
	(register_inferior_data, register_inferior_data_with_cleanup)
	(clear_inferior_data, set_inferior_data, inferior_data): Declare.
	(exit_inferior, exit_inferior_silent, exit_inferior_num_silent)
	(inferior_appeared): Declare.
        (find_inferior_pid): Typo.
        (find_inferior_id, find_inferior_for_program_space): Declare.
	(set_current_inferior, save_current_inferior, prune_inferiors)
	(number_of_inferiors): Declare.
        (inferior_list): Declare.
        * inferior.c: Include "gdbcore.h" and "symfile.h".
        (inferior_list): Make public.
        (delete_inferior_1): Always delete thread silently.
        (find_inferior_id): Make public.
	(current_inferior_): New.
	(current_inferior): Use it.
	(set_current_inferior): New.
	(restore_inferior): New.
	(save_current_inferior): New.
	(free_inferior): Free the per-inferior data.
	(add_inferior_silent): Allocate per-inferior data.
	Call inferior_appeared.
	(delete_threads_of_inferior): New.
	(delete_inferior_1): Adjust interface to take an inferior pointer.
	(delete_inferior): Adjust.
	(delete_inferior_silent): Adjust.
	(exit_inferior_1): New.
	(exit_inferior): New.
	(exit_inferior_silent): New.
	(exit_inferior_num_silent): New.
	(detach_inferior): Adjust.
	(inferior_appeared): New.
	(discard_all_inferiors): Adjust.
	(find_inferior_id): Make public.  Assert pid is not zero.
        (find_inferior_for_program_space): New.
	(have_inferiors): Check if we have any inferior with pid not zero.
	(have_live_inferiors): Go over all pushed targets looking for
	process_stratum.
	(prune_inferiors): New.
	(number_of_inferiors): New.
        (print_inferior): Add executable column.  Print vfork parent/child
	relationships.
	(inferior_command): Adjust to cope with not running inferiors.
	(remove_inferior_command): New.
	(add_inferior_command): New.
	(clone_inferior_command): New.
	(struct inferior_data): New.
	(struct inferior_data_registration): New.
	(struct inferior_data_registry): New.
	(inferior_data_registry): New.
	(register_inferior_data_with_cleanup): New.
	(register_inferior_data): New.
	(inferior_alloc_data): New.
	(inferior_free_data): New.
	(clear_inferior_data): New.
	(set_inferior_data): New.
	(inferior_data): New.
	(initialize_inferiors): New.
	(_initialize_inferiors): Register "add-inferior",
	"remove-inferior" and "clone-inferior" commands.

        * objfiles.h: Include "progspace.h".
        (struct objfile) <pspace>: New field.
        (symfile_objfile, object_files): Don't declare.
        (ALL_PSPACE_OBJFILES): New.
        (ALL_PSPACE_OBJFILES_SAFE): New.
        (ALL_OBJFILES, ALL_OBJFILES_SAFE): Adjust.
        (ALL_PSPACE_SYMTABS): New.
        (ALL_PRIMARY_SYMTABS): Adjust.
        (ALL_PSPACE_PRIMARY_SYMTABS): New.
        (ALL_PSYMTABS): Adjust.
        (ALL_PSPACE_PSYMTABS): New.
        * objfiles.c (object_files, symfile_objfile): Delete.
        (struct objfile_sspace_info): New.
        (objfiles_pspace_data): New.
        (objfiles_pspace_data_cleanup): New.
        (get_objfile_pspace_data): New.
        (objfiles_changed_p): Delete.
        (allocate_objfile): Set the objfile's program space.  Adjust to
        reference objfiles_changed_p in pspace data.
        (free_objfile): Adjust to reference objfiles_changed_p in pspace
        data.
        (objfile_relocate): Ditto.
        (update_section_map): Add pspace argument.  Adjust to iterate over
        objfiles in the passed in pspace.
        (find_pc_section): Delete sections and num_sections statics.
        Adjust to refer to program space's objfiles_changed_p.  Adjust to
        refer to sections and num_sections store in the objfile's pspace
        data.
        (objfiles_changed): Adjust to reference objfiles_changed_p in
        pspace data.
        (_initialize_objfiles): New.
        * linespec.c (decode_all_digits, decode_dollar): Set the sal's
        program space.
        * source.c (current_source_pspace): New.
        (get_current_source_symtab_and_line): Set the sal's program space.
        (set_current_source_symtab_and_line): Set current_source_pspace.
        (select_source_symtab): Ditto.  Use ALL_OBJFILES.
        (forget_cached_source_info): Iterate over all program spaces.
        * symfile.c (clear_symtab_users): Adjust.
        * symmisc.c (print_symbol_bcache_statistics): Iterate over all
        program spaces.
        (print_objfile_statistics): Ditto.
        (maintenance_print_msymbols): Ditto.
        (maintenance_print_objfiles): Ditto.
        (maintenance_info_symtabs): Ditto.
        (maintenance_info_psymtabs): Ditto.
        * symtab.h (SYMTAB_PSPACE): New.
        (struct symtab_and_line) <pspace>: New field.
        * symtab.c (init_sal): Clear the sal's program space.
        (find_pc_sect_symtab): Set the sal's program space.  Switch thread
	and space.
        (append_expanded_sal): Add program space argument.  Iterate over
	all program spaces.
        (expand_line_sal): Iterate over all program spaces.  Switch
	program space.

        * target.h (enum target_waitkind) <TARGET_WAITKIND_VFORK_DONE>: New.
        (struct target_ops) <to_thread_address_space>: New field.
        (target_thread_address_space): Define.
        * target.c (target_detach): Only remove breakpoints from the
        inferior we're detaching.
        (target_thread_address_space): New.

        * defs.h (initialize_progspace): Declare.
        * top.c (gdb_init): Call it.

        * solist.h (struct so_list) <sspace>: New field.
        * solib.h (struct program_space): Forward declare.
        (solib_name_from_address): Adjust prototype.
        * solib.c (so_list_head): Replace with a macro referencing the
        program space.
        (update_solib_list): Set the so's program space.
        (solib_name_from_address): Add a program space argument and adjust.

        * solib-svr4.c (struct svr4_info) <pid>: Delete field.
        <interp_text_sect_low, interp_text_sect_high, interp_plt_sect_low>
        <interp_plt_sect_high>: New fields.
        (svr4_info_p, svr4_info): Delete.
        (solib_svr4_sspace_data): New.
        (get_svr4_info): Rewrite.
        (svr4_sspace_data_cleanup): New.
        (open_symbol_file_object): Adjust.
        (svr4_default_sos): Adjust.
        (svr4_fetch_objfile_link_map): Adjust.
        (interp_text_sect_low, interp_text_sect_high, interp_plt_sect_low)
        (interp_plt_sect_high): Delete.
        (svr4_in_dynsym_resolve_code): Adjust.
        (enable_break): Adjust.
        (svr4_clear_solib): Revert bit that removed the svr4_info here,
        and reinstate clearing debug_base, debug_loader_offset_p,
        debug_loader_offset and debug_loader_name.
        (_initialize_svr4_solib): Register solib_svr4_pspace_data.  Don't
        install an inferior_exit observer anymore.

        * printcmd.c (struct display) <pspace>: New field.
        (display_command): Set the display's sspace.
        (do_one_display): Match the display's sspace.
        (display_uses_solib_p): Ditto.

        * linux-fork.c (detach_fork): Moved to infrun.c.
        (_initialize_linux_fork): Moved "detach-on-fork" command to
        infrun.c.
        * infrun.c (detach_fork): Moved from linux-fork.c.
        (proceed_after_vfork_done): New.
        (handle_vfork_child_exec_or_exit): New.
        (follow_exec_mode_replace, follow_exec_mode_keep)
        (follow_exec_mode_names, follow_exec_mode_string)
        (show_follow_exec_mode_string): New.
        (follow_exec): New.  Reinstate the mark_breakpoints_out call.
        Remove shared libraries before attaching new executable.  If user
        wants to keep the inferior, keep it.
        (displaced_step_fixup): Adjust to pass an address space to the
        breakpoints module.
        (resume): Ditto.
        (clear_proceed_status): In all-stop mode, always clear the proceed
        status of all threads.
        (prepare_to_proceed): Adjust to pass an address space to the
        breakpoints module.
        (proceed): Ditto.
        (adjust_pc_after_break): Ditto.
        (handle_inferior_event): When handling a process exit, switch the
        program space to the inferior's that had exited.  Call
        handle_vfork_child_exec_or_exit.  Adjust to pass an address space
        to the breakpoints module.  In non-stop mode, when following a
        fork and detach-fork is off, also resume the other branch.  Handle
        TARGET_WAITKIND_VFORK_DONE.  Set the program space in sals.
        (normal_stop): Prune inferiors.
        (_initialize_infrun): Install the new "follow-exec-mode" command.
        "detach-on-fork" moved here.

        * regcache.h (get_regcache_aspace): Declare.
        * regcache.c (struct regcache) <aspace>: New field.
        (regcache_xmalloc): Clear the aspace.
        (get_regcache_aspace): New.
        (regcache_cpy): Copy the aspace field.
        (regcache_cpy_no_passthrough): Ditto.
        (get_thread_regcache): Fetch the thread's address space from the
        target, and store it in the regcache.

        * infcall.c (call_function_by_hand): Set the sal's pspace.

        * arch-utils.c (default_has_shared_address_space): New.
        * arch-utils.h (default_has_shared_address_space): Declare.

        * gdbarch.sh (has_shared_address_space): New.
        * gdbarch.h, gdbarch.c: Regenerate.

        * linux-tdep.c: Include auxv.h, target.h, elf/common.h.
        (linux_has_shared_address_space): New.
        (_initialize_linux_tdep): Declare.

        * arm-tdep.c (arm_software_single_step): Pass the frame's address
        space to insert_single_step_breakpoint.
        * arm-linux-tdep.c (arm_linux_software_single_step): Pass the
        frame's pspace to breakpoint functions.
        * cris-tdep.c (crisv32_single_step_through_delay): Ditto.
        (cris_software_single_step): Ditto.
        * mips-tdep.c (deal_with_atomic_sequence): Add frame argument.
        Pass the frame's pspace to breakpoint functions.
        (mips_software_single_step): Adjust.
        (mips_single_step_through_delay): Adjust.
        * rs6000-aix-tdep.c (rs6000_software_single_step): Adjust.
        * rs6000-tdep.c (ppc_deal_with_atomic_sequence): Adjust.
        * solib-irix.c (enable_break): Adjust to pass the current frame's
        address space to breakpoint functions.
        * sparc-tdep.c (sparc_software_single_step): Ditto.
        * spu-tdep.c (spu_software_single_step): Ditto.
        * alpha-tdep.c (alpha_software_single_step): Ditto.
        * record.c (record_wait): Adjust to pass an address space to the
        breakpoints module.

        * fork-child.c (fork_inferior): Set the new inferior's program and
        address spaces.
        * inf-ptrace.c (inf_ptrace_follow_fork): Copy the parent's program
        and address spaces.
        (inf_ptrace_attach): Set the inferior's program and address spaces.
        * linux-nat.c: Include "solib.h".
        (linux_child_follow_fork): Manage parent and child's program and
        address spaces.  Clone the parent's program space if necessary.
        Don't wait for the vfork to be done here.  Refuse to resume if
        following the vfork parent while leaving the child stopped.
        (resume_callback): Don't resume a vfork parent.
        (linux_nat_resume): Also check for pending events in the
        lp->waitstatus field.
        (linux_handle_extended_wait): Report TARGET_WAITKIND_VFORK_DONE
        events to the core.
        (stop_wait_callback): Don't wait for SIGSTOP on vfork parents.
        (cancel_breakpoint): Adjust.
        * linux-thread-db.c (thread_db_wait): Don't remove thread event
        breakpoints here.
        (thread_db_mourn_inferior): Don't mark breakpoints out here.
        Remove thread event breakpoints after mourning.
        * corelow.c: Include progspace.h.
        (core_open): Set the inferior's program and address spaces.
        * remote.c (remote_add_inferior): Set the new inferior's program
        and address spaces.
        (remote_start_remote): Update address spaces.
        (extended_remote_create_inferior_1): Don't init the thread list if
        we already debugging other inferiors.
        * darwin-nat.c (darwin_attach): Set the new inferior's program and
        address spaces.
        * gnu-nat.c (gnu_attach): Ditto.
        * go32-nat.c (go32_create_inferior): Ditto.
        * inf-ttrace.c (inf_ttrace_follow_fork, inf_ttrace_attach): Ditto.
        * monitor.c (monitor_open): Ditto.
        * nto-procfs.c (procfs_attach, procfs_create_inferior): Ditto.
        * procfs.c (do_attach): Ditto.
        * windows-nat.c (do_initial_windows_stuff): Ditto.

	* inflow.c (inferior_process_group)
	(terminal_init_inferior_with_pgrp, terminal_inferior,
	(terminal_ours_1, inflow_inferior_exit, copy_terminal_info)
	(child_terminal_info, new_tty_postfork, set_sigint_trap): Adjust
	to use per-inferior data instead of inferior->terminal_info.
	(inflow_inferior_data): New.
	(inflow_new_inferior): Delete.
	(inflow_inferior_data_cleanup): New.
	(get_inflow_inferior_data): New.

	* mi/mi-interp.c (mi_new_inferior): Rename to...
	(mi_inferior_appeared): ... this.
	(mi_interpreter_init): Adjust.
	
        * tui/tui-disasm.c: Include "progspace.h".
        (tui_set_disassem_content): Pass an address space to
        breakpoint_here_p.

        * NEWS: Mention multi-program debugging support.  Mention new
	commands "add-inferior", "clone-inferior", "remove-inferior",
	"maint info program-spaces", and new option "set
	follow-exec-mode".

gdb/doc/
2009-10-12  Pedro Alves  <pedro@codesourcery.com>
            Stan Shebs  <stan@codesourcery.com>

	* observer.texi (new_inferior): Rename to...
	(inferior_appeared): ... this.
	
gdb/testsuite/
2009-10-12  Pedro Alves  <pedro@codesourcery.com>
            Stan Shebs  <stan@codesourcery.com>

        * gdb.base/foll-vfork.exp: Adjust to spell out "follow-fork".
        * gdb.base/foll-exec.exp: Adjust to expect a process id before
        "Executing new program".
        * gdb.base/foll-fork.exp: Adjust to spell out "follow-fork".
        * gdb.base/multi-forks.exp: Ditto.  Adjust to the inferior being
	left listed after having been killed.
        * gdb.base/attach.exp: Adjust to spell out "symbol-file".
        * gdb.base/maint.exp: Adjust test.

        * Makefile.in (ALL_SUBDIRS): Add gdb.multi.
        * gdb.multi/Makefile.in: New.
        * gdb.multi/base.exp: New.
        * gdb.multi/goodbye.c: New.
        * gdb.multi/hangout.c: New.
        * gdb.multi/hello.c: New.
        * gdb.multi/bkpt-multi-exec.c: New.
        * gdb.multi/bkpt-multi-exec.exp: New.
        * gdb.multi/crashme.c: New.

gdb/doc/
2009-10-12  Pedro Alves  <pedro@codesourcery.com>
            Stan Shebs  <stan@codesourcery.com>

        * gdb.texinfo (Inferiors): Rename node to ...
	(Inferiors and Programs): ... this.  Mention running multiple
	programs in the same debug session.
	
	<info inferiors>: Mention the new 'Executable' column if "info
	inferiors".  Update examples.  Document the "add-inferior",
	"clone-inferior", "remove-inferior" and "maint info
	program-spaces" commands.

        (Process): Rename node to...
        (Forks): ... this.  Document "set|show follow-exec-mode".

-- 
Pedro Alves

[-- Attachment #2: multiexec-v7-20091012.diff.gz --]
[-- Type: application/x-gzip, Size: 81889 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v7] multi-executable support
  2009-10-12  1:26     ` [v7] " Pedro Alves
@ 2009-10-12 19:09       ` Eli Zaretskii
  2009-10-13 13:40         ` [v8] " Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2009-10-12 19:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Mon, 12 Oct 2009 02:26:08 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> Here's the updated patch with docs fixed per review.

Thanks.

> Index: src/gdb/NEWS
> ===================================================================
> --- src.orig/gdb/NEWS	2009-10-11 19:45:41.000000000 +0100
> +++ src/gdb/NEWS	2009-10-11 20:33:58.000000000 +0100
> @@ -3,6 +3,40 @@

This part is fine.

> +may be retained after a process exits.  Inferiors have unique
> +identifiers that are different from process ids.  Usually each
> +inferior will also have its own distinct address space, although some
> +embedded targets may have several inferiors running in different parts
> +of a single address space.  For example, debugging programs that call
> +@code{vfork}, or embedded targets that may have several inferiors
> +running in different parts of a single address space.

The last two sentences say the same about embedded targets in two
slightly different ways.  Suggest to merge into a single sentence.

> +inferior.  Restarting the inferior after the @code{exec} call, with
> +e.g. the @code{run} command, restarts the executable the process was
   ^^^^^
This needs either a @: or a comma after "e.g.", to avoid it being
typeset as an end of a sentence.

Okay with these changes.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [v8] multi-executable support
  2009-10-12 19:09       ` Eli Zaretskii
@ 2009-10-13 13:40         ` Pedro Alves
  2009-10-14  3:03           ` Joel Brobecker
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2009-10-13 13:40 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

On Monday 12 October 2009 20:11:23, Eli Zaretskii wrote:
> The last two sentences say the same about embedded targets in two
> slightly different ways.  Suggest to merge into a single sentence.

Done.

> > +inferior.  Restarting the inferior after the @code{exec} call, with
> > +e.g. the @code{run} command, restarts the executable the process was
>    ^^^^^
> This needs either a @: or a comma after "e.g.", to avoid it being
> typeset as an end of a sentence.

Thanks.  Fixed.

> Okay with these changes.

Thank you very much for all the helpful reviews.

Here's the final patch I'm proposing to commit.

Anyone else would like to comment?  Otherwise, I'll aim at
applying this at the end of the week, say.

-- 
Pedro Alves

[-- Attachment #2: multiexec-v8-20091013.diff.gz --]
[-- Type: application/x-gzip, Size: 81854 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-13 13:40         ` [v8] " Pedro Alves
@ 2009-10-14  3:03           ` Joel Brobecker
  2009-10-14 15:16             ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2009-10-14  3:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro,

I finally had some time to read through the lengthy threads for this.
I'll confess that I didn't look at the code much because I'm starved
for time and I trust you implicitly. I just looked at the discussions
and the resulting UI that you now propose.

I like the UI, and the concepts behind it. I did a careful reading of
the introductory comment in progspace.h and it's great that you took
the time to provide such a detailed introduction.  I'm planning on
extending it to discuss the VxWorks case :-).

Just a few nits: There is at least one location where I noticed some
"#if 0"'ed code, and I just told one of the contributors that we'd
like to avoid this sort of thing.  Can it be removed, or should we
really be keeping that piece of commented out code.  We usually use
FIXMEs instead.

In the documentation:

    +(@value{GDBP}) clone-inferior
    +Added inferior 2.
    +1 inferiors added.

This is really nit-picking, but when adding 1 inferior, should we have
a special message that does not say "inferiors" (plural). I usually
don't care much about this sort of detail, but for some reason, it
seems to matter to me in this case.

I am very impressed with the level of functionality that this patch
is bringing to GDB, and I look forward to seeing it part of GDB.

Just a personal request: If you commit the patch before Saturday,
could you just give me a heads up. I just want to make sure that
I can get a checkout of the sources before you apply your patch.
I think that the patch is going to break our VxWorks port, and I think
that fixing it will require a bit of thoughts and time.  So if I can
do a resync of the AdaCore sources up to before your patch, that'll
give me a little more time.

Cheers,
-- 
Joel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-14  3:03           ` Joel Brobecker
@ 2009-10-14 15:16             ` Pedro Alves
  2009-10-14 15:28               ` Joel Brobecker
  2009-10-14 16:25               ` Tom Tromey
  0 siblings, 2 replies; 15+ messages in thread
From: Pedro Alves @ 2009-10-14 15:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wednesday 14 October 2009 04:03:10, Joel Brobecker wrote:

> I like the UI, and the concepts behind it. I did a careful reading of
> the introductory comment in progspace.h and it's great that you took
> the time to provide such a detailed introduction.  I'm planning on
> extending it to discuss the VxWorks case :-).
> 

Great!

> Just a few nits: There is at least one location where I noticed some
> "#if 0"'ed code, and I just told one of the contributors that we'd
> like to avoid this sort of thing.  Can it be removed, or should we
> really be keeping that piece of commented out code.  We usually use
> FIXMEs instead.

Ah, yeah, I'll drop that for now.

> 
> In the documentation:
> 
>     +(@value{GDBP}) clone-inferior
>     +Added inferior 2.
>     +1 inferiors added.
> 
> This is really nit-picking, but when adding 1 inferior, should we have
> a special message that does not say "inferiors" (plural). I usually
> don't care much about this sort of detail, but for some reason, it
> seems to matter to me in this case.

Any suggestions?  Assuming languages have only singular and one
plural form [if (n == 1) else ...] is frowned uppon as well.

 "1 inferior(s) added." ?

 "Added 1 inferior(s)." ?

 Just drop the sentence?

> Just a personal request: If you commit the patch before Saturday,
> could you just give me a heads up. I just want to make sure that
> I can get a checkout of the sources before you apply your patch.
> I think that the patch is going to break our VxWorks port, and I think
> that fixing it will require a bit of thoughts and time.  

Oh.  Don't hesitate to ask if you have any question.  I hope the
models can be matched without much trouble.

> So if I can 
> do a resync of the AdaCore sources up to before your patch, that'll
> give me a little more time.

Sure, no problem.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-14 15:16             ` Pedro Alves
@ 2009-10-14 15:28               ` Joel Brobecker
  2009-10-14 16:25               ` Tom Tromey
  1 sibling, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2009-10-14 15:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Any suggestions?  Assuming languages have only singular and one
> plural form [if (n == 1) else ...] is frowned uppon as well.
> 
>  "1 inferior(s) added." ?
> 
>  "Added 1 inferior(s)." ?
> 
>  Just drop the sentence?

Hmmm, I didn't realize that [if (n == 1) else ...] was also frowned
upon. Not sure how this issue is usually solved, but in this case
I think we can indeed drop the sentence entirely.  Otherwise, if
people want to keep it, then either form looks fine.

> Oh.  Don't hesitate to ask if you have any question.  I hope the
> models can be matched without much trouble.

I sure will :)

> > So if I can do a resync of the AdaCore sources up to before your
> > patch, that'll give me a little more time.
> 
> Sure, no problem.

Actually, not to worry after all. I modified my procedures to use
the git repo instead, and it's very easy to go back, so cancel that
personal request :).

(after so many months of using git, I still cannot believe how much
of a lifesaver it is - I keep discovering new ways to improve my work
with it, and I simply have no time to get used to it!)

-- 
Joel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-14 15:16             ` Pedro Alves
  2009-10-14 15:28               ` Joel Brobecker
@ 2009-10-14 16:25               ` Tom Tromey
  2009-10-14 16:48                 ` Pedro Alves
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2009-10-14 16:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Any suggestions?  Assuming languages have only singular and one
Pedro> plural form [if (n == 1) else ...] is frowned uppon as well.
Pedro>  "1 inferior(s) added." ?
Pedro>  "Added 1 inferior(s)." ?
Pedro>  Just drop the sentence?

gettext has support for this.  See (info "(gettext)Plural Forms"),
specifically the "ngettext" function.

In this case I think you would write:

  printf_filtered (ngettext ("%d inferior added.\n", "%d inferiors added.\n",
			     copies),
		   copies);

Glancing at the patch it seems to be missing some _() calls in this
area.

Tom


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-14 16:25               ` Tom Tromey
@ 2009-10-14 16:48                 ` Pedro Alves
  2009-10-14 17:41                   ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2009-10-14 16:48 UTC (permalink / raw)
  To: gdb-patches, tromey; +Cc: Joel Brobecker

On Wednesday 14 October 2009 17:24:21, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> Any suggestions?  Assuming languages have only singular and one
> Pedro> plural form [if (n == 1) else ...] is frowned uppon as well.
> Pedro>  "1 inferior(s) added." ?
> Pedro>  "Added 1 inferior(s)." ?
> Pedro>  Just drop the sentence?
> 
> gettext has support for this.  See (info "(gettext)Plural Forms"),
> specifically the "ngettext" function.
> 
> In this case I think you would write:
> 
>   printf_filtered (ngettext ("%d inferior added.\n", "%d inferiors added.\n",
> 			     copies),
> 		   copies);
> 

Thanks, had seen that.  I had noticed that gdb_locale.h doesn't wrap
ngettext, and GDB doesn't use it anywhere, so I just perhaps wrongly
assumed that ngettext would be missing on some non-GNU gettext
implementations.

In any case, I'm not concerned that users will miss the
sentence, so I'll just drop it.

> Glancing at the patch it seems to be missing some _() calls in this
> area.

Eh, indeed.  I'll go through the patch to see if more are
missing.  Thanks.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v8] multi-executable support
  2009-10-14 16:48                 ` Pedro Alves
@ 2009-10-14 17:41                   ` Tom Tromey
  0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2009-10-14 17:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Joel Brobecker

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Thanks, had seen that.  I had noticed that gdb_locale.h doesn't wrap
Pedro> ngettext, and GDB doesn't use it anywhere, so I just perhaps wrongly
Pedro> assumed that ngettext would be missing on some non-GNU gettext
Pedro> implementations.

Yeah, I don't know about non-GNU implementations.  Is there any reason
not to simply always use the GNU one?  I really don't know any more.

Tom


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2009-10-14 17:41 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-05 15:59 [v6] multi-executable support Pedro Alves
2009-10-05 21:57 ` Eli Zaretskii
2009-10-06 12:14   ` Pedro Alves
2009-10-06 14:29     ` Eli Zaretskii
2009-10-12  1:26     ` [v7] " Pedro Alves
2009-10-12 19:09       ` Eli Zaretskii
2009-10-13 13:40         ` [v8] " Pedro Alves
2009-10-14  3:03           ` Joel Brobecker
2009-10-14 15:16             ` Pedro Alves
2009-10-14 15:28               ` Joel Brobecker
2009-10-14 16:25               ` Tom Tromey
2009-10-14 16:48                 ` Pedro Alves
2009-10-14 17:41                   ` Tom Tromey
2009-10-06  5:13 ` [v6] " Sérgio Durigan Júnior
2009-10-06 13:55   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox