Re: [RFA] target adjust pc after break with itself

[teawater <teawater@gmail.com>]

Hi,

On Wed, Nov 5, 2008 at 02:32, Pedro Alves <pedro@codesourcery.com> wrote:
>
> Hi,
>
> On Sunday 02 November 2008 06:01:29, teawater wrote:
> > Hi Pedro,
> >
> > According to your idea, I write a patch
> > "target_adjust_pc_with_itself.txt" for it. It's for the main trunk and
> > 20080930 branch.
> >
>
> Could we place this in the branch only for a while and let it cook
> there?  There's nothing in mainline that would require this for
> now, unless Michael wants to do something similar for his remote target.
>

OK.

> Your patch will do for target record, though there are several other
> places that check gdbarch_decr_pc_after_break () outside of infrun
> that should be adjusted --- but no other target cares currently.
> Maybe we should centralize this a bit more, so a single call would
> be needed everywhere else instead
> of (if !target_adjusts_pc_... && gdbarch_decr_pc...))
>

Sorry for my careless. I will fix it in the next patch.

> I'm always thinking about target remote when I add new target_ops
> interfaces, because that's the case where the decoupling is as high
> as it comes; with that in mind, I don't think that there'll be cases
> were the gdbarch does decr_pc_after_break == 0, while the target will
> want to override it to != 0; this interface is probably fine.
>

Cool. I will add a interface for it too. It will called
"target_decr_pc_after_break". If "target_adjusts_pc_after_break" is
true, GDB will use it instead "gdbarch_decr_pc_after_break".

> I'm not a native speaker as well, but "with itself" doesn't
> sound right to me.  How about something like:
>
>  target_adjusts_pc_after_break ()
>
>  if (target_adjusts_pc_after_break ())
>   return;  /* ... then we don't have to.  */
>
> or
>
>  target_adjusts_pc_after_break_itself
>
> ?

I will change it to "target_adjusts_pc_after_break".


Thanks for your advices. I will make a new patch for it.

Thanks,
Hui


>
> > 2008-11-02  Hui Zhu  <teawater@gmail.com>
> >
> >       * target.h (target_ops): Add "to_adjust_pc_with_itself".
> >       Return true if target adjust pc after break with itself.
> >       (target_adjust_pc_with_itself): New macro.
> >       Call "to_adjust_pc_with_itself".
> >       * target.c (update_current_target): Set
> >       "to_adjust_pc_with_itself".
> >       * infrun.c (adjust_pc_after_break): If
> >       "target_adjust_pc_with_itself" return true, not adjust pc.
> >
> > To make P record support it. I make another patch
> > "record_adjust_pc_with_itself.txt".
> >
> > 2008-11-02  Hui Zhu  <teawater@gmail.com>
> >
> >       * record.c (record_adjust_pc_with_itself): New function.
> >       Return true.
> >       (init_record_ops): Set to_adjust_pc_with_itself point to
> >       record_adjust_pc_with_itself.
>
> --
> Pedro Alves