Should push_target pop all targets?

Should push_target pop all targets?

[Andrew Cagney]

Hello,

The current push_target code:

>   /* Find the proper stratum to install this target in. */
> 
>   for (prev = NULL, cur = target_stack; cur; prev = cur, cur = cur->next)
>     {
>       if ((int) (t->to_stratum) >= (int) (cur->target_ops->to_stratum))
>         break;
>     }
> 
>   /* If there's already targets at this stratum, remove them. */

only pops targets at the stratum level being pushed.  Doing this - 
changing the target underneath ones feet - just scares me.

I think this should be changed so that all targets above that one get 
poped.  If they are still needed, they can be re-pushed.

I suspect this will affect core-file and thread targets ...
Andrew

Re: Should push_target pop all targets?

[Daniel Jacobowitz]

On Wed, Oct 15, 2003 at 06:41:54PM -0400, Andrew Cagney wrote:
> Hello,
> 
> The current push_target code:
> 
> >  /* Find the proper stratum to install this target in. */
> >
> >  for (prev = NULL, cur = target_stack; cur; prev = cur, cur = cur->next)
> >    {
> >      if ((int) (t->to_stratum) >= (int) (cur->target_ops->to_stratum))
> >        break;
> >    }
> >
> >  /* If there's already targets at this stratum, remove them. */
> 
> only pops targets at the stratum level being pushed.  Doing this - 
> changing the target underneath ones feet - just scares me.
> 
> I think this should be changed so that all targets above that one get 
> poped.  If they are still needed, they can be re-pushed.
> 
> I suspect this will affect core-file and thread targets ...

This definitely makes sense to me.  I suspect a lot of the uses of
push/pop target will need some examining, though, based on my last
wander through that code.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Should push_target pop all targets?

[Andrew Cagney]

> On Wed, Oct 15, 2003 at 06:41:54PM -0400, Andrew Cagney wrote:
> 
>> Hello,
>> 
>> The current push_target code:
>> 
> 
>> >  /* Find the proper stratum to install this target in. */
>> >
>> >  for (prev = NULL, cur = target_stack; cur; prev = cur, cur = cur->next)
>> >    {
>> >      if ((int) (t->to_stratum) >= (int) (cur->target_ops->to_stratum))
>> >        break;
>> >    }
>> >
>> >  /* If there's already targets at this stratum, remove them. */
> 
>> 
>> only pops targets at the stratum level being pushed.  Doing this - 
>> changing the target underneath ones feet - just scares me.
>> 
>> I think this should be changed so that all targets above that one get 
>> poped.  If they are still needed, they can be re-pushed.
>> 
>> I suspect this will affect core-file and thread targets ...
> 
> 
> This definitely makes sense to me.  I suspect a lot of the uses of
> push/pop target will need some examining, though, based on my last
> wander through that code.

On GNU/Linux, attach breaks (yet the exact same test passes on BSD ...)

Andrew

Re: Should push_target pop all targets?

[Andrew Cagney]

> On GNU/Linux, attach breaks (yet the exact same test passes on BSD ...)

I believe this happens:

(gdb) attach pid
-> calls attach_command()
    -> calls ... find_default_run_target's attach
    -> calls exec_file_attach()
       -> calls push_target (&exec_ops)
          pops the "run" target

outch!

Andrew