Re: [PATCH] add -s option to make -break-insert support dprintf

Pedro Alves
2013-04-10T10:48:18Z

Hi Marc,

On 04/10/2013 11:31 AM, Marc Khouzam wrote:

> In the orginal patch, having both 'hardware' and 'dprintf' true would
> create a hardware breakpoint (not dprintf), but would still set 'ops'
> to &dprintf_breakpoint_ops. This didn't look right to me.

In fairness, it actually wouldn't, because of:

>>> + if (hardware && dprintf)
>>> + error (_("-break-insert: -h and -s cannot be use together"));

I guess my change at least makes it more obvious in that other
spot too.

> A side-effect of Pedro's change is that the hardware dprintf case
> will be handled properly. I think that is a good thing. However,
> I wanted to mention it, as I don't know if there are other changes
> needed to handle a hardware dprintf (or if it really should be allowed).
> I am allowed to create a hardware breakpoint with a printf condition,
> so I guess a hardware dprintf would make sense, but I'm not sure.

Yeah, I don't see why it couldn't work. Any kind of
watchpoint/breakpoint, really. To me, this exposes that the whole
dprintf feature hasn't been exposed to the user at the right
abstraction level. It should have been something lower level that
allowed constructing dprintfs, or hardware-dprintfs, or
watchpoints-dprintfs, etc. The python Stop hook is close, except
for the agent side part. Maybe equipping breakpoints with a separate
list of "commands that don't interfere with the current execution
command at time of hit" would have been closer to home.

--
Pedro Alves