Re: [rfc target-side break conditions 3/5 v2] GDB-side changes

[Stan Shebs <stanshebs at earthlink dot net>]

On 1/27/12 12:33 PM, Luis Gustavo wrote:
> A few more changes on this patch. It addresses all the previous 
> comments and suggestions.
>
>
> I've changed the "info break" output a little. We now display "<mode> 
> evaluates conditions" next to the location description, and we also 
> output that same information via MI. Though it works, i'm not entirely 
> happy with the way it is displayed.
>
> Previously i displayed such information next to the condition field, 
> but it doesn't work right for breakpoints with multiple locations 
> since the conditions are printed for the breakpoint instead of the 
> locations. Suggestions?

Can it actually happen that some locations of a breakpoint are evaluated 
host-side, and other target-side?  It makes my head hurt trying to think 
of an example. :-)

In any case, the note should follow the condition, and I suggest the 
wording "(host evals)" and "(target evals)" if all locations are going 
to be handled the same way.  If locations are varying, then the 
condition follower can say "(host or target evals)" then each location 
description can say just "(host)" or "(target)".  It's a little messier 
to calculate, but users will appreciate the clarity.

(I'm preferring "host" to "gdb", but I suppose that might be confusing 
if GDB was talking to a sprite also running on the host and controlling 
target via JTAG.)

+/* Mark locations as "conditions have changed" in case
+   the target supports evaluating conditions on
+   its side.  */


This is kind of petty, but shouldn't the lines be filled?  I think Emacs would make this a two-line comment.


+      warning (_("Target does not support condition evaluation.\n"
+		 "Using GDB evaluation mode instead."));


I tend to think this should say "breakpoint condition", so it's clear that it's not referring to tracepoint conditions, or command list conditions, or Python conditions, etc.

+      /* If we got here, it means the condition could not be parse to a valid


be parsed

(Eventually we should be nicer and say *why* the condition didn't make the cut. Users would be happy to replace a convenience var with a literal value if they knew that's all that was wrong.)

-  update_global_location_list (0);
+  if (is_breakpoint (bpt))
+    update_global_location_list (1);
+  else
+    update_global_location_list (0);


   update_global_location_list (is_breakpoint (bpt))


Stan