This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA 2/3] Demote to sw watchpoint only in update_watchpoint


Thiago Jung Bauermann wrote:

> I was trying to adapt the logic in both places for masked watchpoints,
> but it's hard to anticipate in watch_command_1 what update_watchpoint
> will ultimately do without duplicating a good portion of the latter. The
> code in watch_command_1 was getting complicated and hard to get right in
> all situations for something which would be done again later anyway.

Agreed, that's a good idea.

> I decided that it was cleaner if watch_command_1 just delegated
> everything to update_watchpoint. This patch makes it create a
> watchpoint, call update_watchpoint and then delete it if some error is
> hit. The only drawback I can think of is that the aborted watchpoint
> will "consume" one breakpoint number, and thus GDB will skip one number
> when creating the next breakpoint/watchpoint. This doesn't seem
> important to me.

Hmm, I don't really like that change.  However, it should be easy to
fix by just moving the set_breakpoint_number call to down below where
update_watchpoint has succeeded, shouldn't it?

> 	(delete_breakpoint): Add announce argument to control whether
> 	observers are notified of the deletion.  Update all callers.

Ugh.  Adding an extra argument that everybody must be aware of just
to take care of this special case isn't really nice ...  I'd prefer
if you'd either split off the parts of delete_breakpoint that are
needed to undo partial creation into a separate function, or else,
if you use the set_breakpoint_number trick, identify partial
breakpoints by the fact that their b->number is still zero.

> @@ -1432,8 +1436,22 @@ update_watchpoint (struct breakpoint *b, int reparse)
>  	      target_resources_ok = target_can_use_hardware_watchpoint
>  		    (bp_hardware_watchpoint, i, other_type_used);
This should now use b->type instead of hardcoding bp_hardware_watchpoint,
shouldn't it?

Otherwise looks good to me.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]