This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Add visible flag to breakpoints.
>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
Phil> In this patch iteration I removed any form of storage/tracking of
Phil> breakpoints. Instead, as above, I placed a reference in the breakpoint
Phil> struct.
I like this better.
Phil> There are two disadvantages that stem from this:
Phil> * I had to extern breakpoint_chain
Phil> * I had to move the ALL_BREAKPOINTS macro to breakpoint.h
Phil> I'm not sure how to get around those, or, if indeed they are perceived
Phil> as disadvantages.
It is definitely bad to do this. I think it is fixable, though.
It is a little odd that the observer passes the number and not the
breakpoint itself.
Phil> + if (internal && PyObject_IsTrue (internal))
Phil> + internal_bp = 1;
PyObject_IsTrue can return -1 on failure, so this code must account for
that.
I see we have a case where we don't check this in py-prettyprint.c :(
I will fix that.
Phil> + int i = 0;
Phil> + ALL_BREAKPOINTS (b)
This instance of ALL_BREAKPOINTS should probably be replaced with some
kind of callback API, like the other iterate_over_* functions in gdb.
Phil> + {
Phil> + /* Not all breakpoints will have a companion Python object.
Phil> + Only breakpoints that were created via bppy_new, or
Phil> + breakpoints that were created externally and are tracked by
Phil> + the Python Scripting API. */
Phil> + if (b->py_bp_object)
Phil> + {
Phil> + if (PyTuple_SetItem (result, i, (PyObject *) b->py_bp_object) != 0)
I think that cast to PyObject is unnecessary.
Phil> + {
Phil> + Py_DECREF (result);
Phil> + return NULL;
Phil> + }
Phil> + Py_INCREF (b->py_bp_object);
This incref should come before the call to PyTuple_SetItem.
Phil> @@ -667,9 +673,7 @@ gdbpy_breakpoint_created (int num)
Phil> breakpoint_object *newbp;
Phil> struct breakpoint *bp = NULL;
Phil> PyGILState_STATE state;
Phil> -
Phil> - if (num < 0)
Phil> - return;
Phil> + int error = 0;
I am not certain that we want a new breakpoint object to be created for
internal breakpoints set by other modules. It seems potentially
harmful.
I think one way to do this would be to rephrase this check as
if (num < 0 && bppy_pending_object == NULL)
Phil> + newbp->bp->py_bp_object = (PyObject *)newbp;
No cast.
Phil> + if (! error)
Phil> + ++bppy_live;
You can get rid of the 'error' local and just push this back up into the
"ok" branch of the preceding 'if'.
Phil> /* Just ignore errors here. */
Phil> PyErr_Clear ();
I think this is redundant now.
Phil> + ALL_BREAKPOINTS (b)
Phil> + {
Phil> + if (b->number == num)
Phil> + {
Phil> + breakpoint_object *bp_obj =
Phil> + ((breakpoint_object *)b->py_bp_object);
I think you can replace this ALL_BREAKPOINTS with a call to
get_breakpoint.
Space after the close paren in the cast.
Tom