Re: [PATCH v9 09/10] btrace, python: Enable ptwrite filter registration.

[Tom Tromey <tom@tromey.com>]

>>>>> Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:

> By default GDB will be printing the hex payload of the ptwrite package as
> auxiliary information.  To customize this, the user can register a ptwrite
> filter function in python, that takes the payload and the PC as arguments and
> returns a string which will be printed instead.  Registering the filter
> function is done using a factory pattern to make per-thread filtering easier.

Thank you for the patch.

> +  /* Function pointer to the ptwrite callback.  Returns the string returned
> +     by the ptwrite filter function.  */
> +  gdb::optional<std::string> (*ptw_callback_fun) (const uint64_t payload,
> +						  const uint64_t ip,
> +						  const void *ptw_context)
> +						    = nullptr;
> +
> +  /* Context for the ptw_callback_fun.  */
> +  void *ptw_context = nullptr;

I tend to think it's better to use std::function here, rather than the
C-style pointer-and-data approach.

> diff --git a/gdb/python/lib/gdb/ptwrite.py b/gdb/python/lib/gdb/ptwrite.py

I gather the new stuff is documented in the next patch.

> +# _ptwrite_filter contains the per thread copies of the filter function.
> +# The keys are tuples of inferior id and thread id.
> +# The filter functions are created for each thread by calling the
> +# _ptwrite_filter_factory.
> +_ptwrite_filter = {}

It seems like when an inferior or thread is destroyed, the entries
should be removed from this map.

> +  gdbpy_ref<> py_payload (PyLong_FromUnsignedLongLong (payload));
> +  gdbpy_ref<> py_ip (PyLong_FromUnsignedLongLong (ip));

Both of these calls require null checks.

> +  gdbpy_ref<> py_result (PyObject_CallFunctionObjArgs ((PyObject *) ptw_filter,
> +							py_payload.get (),
> +							py_ip.get (),
> +							nullptr));
> +
> +  if (PyErr_Occurred ())

It's more idiomatic to check == nullptr than PyErr_Occurred.

> +  result = gdbpy_obj_to_string (py_result.get ()).get ();
> +
> +  if (PyErr_Occurred ())

Here too.

Tom