Re: [patch] Support inferior events in python

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> Added. I also, documented StopEvent. In events which inherit from
Sami> others I added a statement asking the reader to refer to the
Sami> parent. Do you think that is enough, or should the inherited
Sami> attributes be restated ?

I think it is sufficient to mention the superclass.

Sami> +int
Sami> +emit_stop_event (struct bpstats *bs, enum target_signal stop_signal)
Sami> +{
Sami> +  PyObject *stop_event_obj = NULL; /* Appease GCC warning.  */
Sami> +
Sami> +  if (evregpy_no_listeners_p (gdb_py_events.stop))
Sami> +    return 0;

Sami> +  if (bs && bs->breakpoint_at
Sami> +      && bs->breakpoint_at->type == bp_breakpoint)

I think this logic is too restrictive.
What if we change what breakpoints are reflected to Python?
That is, I think the check for bp_breakpoint should be removed.
Instead, you could just have it check ->py_bp_object.
But if you are doing that you should either rearrange, or just get rid
of gdbpy_breakpoint_from_bpstats, to either preserve the abstraction or
remove it.

Sami> +      PyObject *breakpoint = gdbpy_breakpoint_from_bpstats (bs);
Sami> +      if (breakpoint != NULL)
Sami> +      stop_event_obj =
Sami> +	  create_breakpoint_event_object (breakpoint);
Sami> +      if (!stop_event_obj)
Sami> +	goto fail;

This could result in a "fail" without an error being set.
But the above change should eliminate that.

Sami> +PyObject *
Sami> +create_thread_event_object (PyTypeObject *py_type)
Sami> +{
Sami> +  PyObject *thread_event_obj = create_event_object (py_type);
Sami> +  PyObject *thread = get_event_thread();

Missing space.

Sami> +
Sami> +  if (!thread_event_obj || !thread)
Sami> +    goto fail;

You have to do error checks after each Python call, you can't batch them.

Either: create_event_object can fail, in which case it is not ok to call
get_event_thread on failure; or, create_event_object can return NULL but
not set a Python exception, in which case you must set one explicitly.

Tom