This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix create pending breakpoint handle extra_string issue if not parse_condition_and_thread
- From: Keith Seitz <keiths at redhat dot com>
- To: Hui Zhu <hui_zhu at mentor dot com>
- Cc: Tom Tromey <tromey at redhat dot com>, Hui Zhu <teawater at gmail dot com>, gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Mon, 25 Mar 2013 11:09:54 -0700
- Subject: Re: [PATCH] Fix create pending breakpoint handle extra_string issue if not parse_condition_and_thread
- References: <514E8D6C dot 2010606 at mentor dot com> <514EEB43 dot 6040101 at redhat dot com> <CANFwon3rNhdJaaYr1UDy3XJTY2-s5N74QB_LmgVi2d-Qcp4FWQ at mail dot gmail dot com> <87620ftn9f dot fsf at fleche dot redhat dot com>
On 03/25/2013 10:14 AM, Tom Tromey wrote:
"Hui" == Hui Zhu <teawater@gmail.com> writes:
Hui> I am sorry that what you care about is the issue that affect the mi.
Hui> But my patch is for the issue inside the function create_breakpoint.
Actually I /was/ talking about create_breakpoint. As you stated, the
only way to demonstrate the problem is via MI, so that's what I used to
demonstrate how I think the situation should be handled.
Here's a patch which does exactly what I consider the "right" way to
react to having both cond_string and a condition inside arg:
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.747
diff -u -p -r1.747 breakpoint.c
--- breakpoint.c 20 Mar 2013 22:17:18 -0000 1.747
+++ breakpoint.c 25 Mar 2013 17:59:36 -0000
@@ -9659,6 +9659,11 @@ create_breakpoint (struct gdbarch *gdbar
extra_string = xstrdup (extra_string);
make_cleanup (xfree, extra_string);
}
+ else if (*arg != '\000')
+ {
+ extra_string = xstrdup (arg);
+ make_cleanup (xfree, extra_string);
+ }
}
ops->create_breakpoints_sal (gdbarch, &canonical, lsal,
In this case, it seems to me that the API must be a bad one.
Yes, that API extension was a horribly implemented (quick and dirty),
but create_breakpoint is a bit of a mess, since it not only has to deal
with setting breakpoints (of various varieties), it also has to deal
with parsing user input. I'm not a fan of this (too common) paradigm.
Can't we just tell callers, "don't do that"?
To me it seems like a pathological case.
We can certainly enforce this, as my patchlet above demonstrates:
-break-insert -c "argc > 1" "main if argc > 2"
^error,msg="Garbage 'if argc > 2' at end of command"
Keith