Re: [patch] Add an evaluation function hook to Python breakpoints.

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 13 December 2010 13:50:36, Phil Muldoon wrote:

> +      /* Evaluate Python breakpoints that have an "evaluate"
> +        function implemented.  */
> +#if HAVE_PYTHON
> +      if (b->py_bp_object)
> +       {
> +         struct cleanup *cleanup = ensure_python_env (get_current_arch (),
> +                                                      current_language);
> +         PyObject *gdbpy_bp_eval = PyString_FromString ("evaluate");
> +         PyObject *py_bp = (PyObject *) b->py_bp_object;
> +
> +         if (PyObject_HasAttr (py_bp, gdbpy_bp_eval))
> +           {
> +             PyObject *result = PyObject_CallMethodObjArgs (py_bp,
> +                                                            gdbpy_bp_eval,
> +                                                            NULL);
> +
> +             if (result)
> +               {
> +                 int evaluate = PyObject_IsTrue (result);
> +
> +                 if (evaluate == -1)
> +                   gdbpy_print_stack ();
> +
> +                 /* If the evaluate function returns False that means the
> +                    Python breakpoint wants GDB to continue.  */
> +                 if (!evaluate)
> +                   bs->stop = 0;
> +               }
> +             else
> +               gdbpy_print_stack ();
> +           }
> +         do_cleanups (cleanup);
> +       }
> +#endif
> +

Can you factor out the PyObject manipulations and the actual evaluation
of the condition to pythong/py-breakpoint.c?  Say, to a
new "py_breakpoint_evaluate (struct breakpoint_object *, ...)" function.
The driving idea being to get rid of the need to now include
python-internal.h.

My first reaction was 'why not call the field "condition"?  "evaluate"
sounds like it's about watchpoint evaluation or some such to me.  Point
being, that there are several different things that are evaluated, and
so it kind of sounds ambiguous.  OTOH, there's chance of confusion with
the condition expression set with the "condition" command.  Is that one
exposed to python?  It may be worth it to think a bit about that, so to
make sure the docs and api doesn't end up confusing when you end up
exposing that condition too.  I'm okay with whatever you guys come up
with, just pointing it out.

-- 
Pedro Alves