Re: [patch] Simplify breakpoint.c function parameters

[Vladimir Prus <vladimir@codesourcery.com>]

Jan Kratochvil wrote:

> Hi,
> 
> the patch has no effect on the functionality but I find the simplified code
> less magic to understand.
> 
> It fixes a small bug - update_watchpoint() for bp_watchpoint (software
> watchpoint) created breakpoint locations bp_loc_hardware_watchpoint 

update_watchpoint() is documented thusly:

        /* Assuming that B is a hardware watchpoint:
           - Reparse watchpoint expression, is REPARSE is non-zero
           - Evaluate expression and store the result in B->val
           - Update the list of values that must be watched in B->loc.

           If the watchpoint is disabled, do nothing.  If this is
           local watchpoint that is out of scope, delete it.  */
        static void
        update_watchpoint (struct breakpoint *b, int reparse)

but it appears to be actually called from breakpoint_re_set_one for the
software watchpoint, too. I suspect it's a bug -- for software watchpoint
we don't have to get a list of values to watch, which update_watchpoint()
does more or less unconditionally. But this bug might be benign -- we probably
never actually insert the computed locations.

> while it 
> should be bp_loc_other.  But I am not aware of any (possibly negative) effect
> it had.
> 
> No regressions on {x86_64,ia64}-unknown-linux-gnu.

FWIW, this sounds reasonable and safe to me.

- Volodya