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

[Pedro Alves <pedro@codesourcery.com>]

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.

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...))

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.

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

?

> 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