Re: RFC: remove pop_target

[Pedro Alves <palves at redhat dot com>]

On 07/02/2013 08:35 PM, Tom Tromey wrote:
> This patch fixes the target double-close problem (PR remote/15266),
> and in the process removes pop_target entire (PR remote/15256).
> 
> The first issue is that pop_target calls target_close.  However, it
> then calls unpush_target, which also calls target_close.  This means
> targets must be able to be closed twice.  Not only is this strange,

Yeah, IMO, a relic of when target_close took a QUITTING argument:

pop_target (void)
{
  target_close (target_stack);		/* Let it clean up.  */
  if (unpush_target (target_stack) == 1)

The "Let it clean up" comment still hints at it.  This particular
target_close call could behave differently from the one in
unpush_target.  But that's gone now.

> Finally, this adds an assertion to target_close to ensure that no
> currently-pushed target can be closed.

Good idea.  You don't mention, but I see the target_bfd_reopen change
is related to that.

> 
> Built and regtested on x86-64 Fedora 18; both natively and using the
> native-gdbserver board file.
> 
> 	PR remote/15256, PR remote/15266:
> 	* bfd-target.c (target_bfd_reopen): Initialize to_magic.
> 	* monitor.c (monitor_detach): Use unpush_target.
> 	* remote-m32r-sdi.c (m32r_detach): Use unpush_target.
> 	* remote-mips.c (mips_detach): Use unpush_target.  Don't
> 	call mips_close.
> 	* remote-sim.c (gdbsim_detach): Use unpush_target.
> 	* target.c (pop_target): Remove.
> 	(pop_all_targets_above): Don't call target_close.
> 	(target_close): Assert that the target is unpushed.
> 	* target.h (pop_target): Don't declare.
> 	* tracepoint.c (tfile_open): Use unpush_target.

This looks good to me.

> --- a/gdb/monitor.c
> +++ b/gdb/monitor.c
> @@ -877,7 +877,7 @@ monitor_close (void)
>  static void
>  monitor_detach (struct target_ops *ops, char *args, int from_tty)
>  {
> -  pop_target ();		/* calls monitor_close to do the real work.  */
> +  unpush_target (ops);

Nit, but you've preserved the similar comment in the other hunks.

-- 
Pedro Alves