Re: [PATCH v3 2/7] Add `thread_from_thread_handle' method to (Python) gdb.Inferior

[Simon Marchi <simon dot marchi at polymtl dot ca>]

Hi Kevin,

On 2017-07-19 02:54, Kevin Buettner wrote:
> gdb/ChangeLog:
>     	* python/py-inferior.c (gdbpy_thread_from_thread_handle): New
>     	function.
>     	(inferior_object_methods): Add gdbpy_thread_from_thread_handle.
>     	* python/python-internal.h (thread_object_type): Declare.
> ---
>  gdb/python/py-inferior.c     | 47 
> ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h |  2 ++
>  2 files changed, 49 insertions(+)
>
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index f6a24a0..082ded0 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -751,6 +751,49 @@ infpy_is_valid (PyObject *self, PyObject *args)
>    Py_RETURN_TRUE;
>  }
>
> +/* Implementation of gdb.Inferior.thread_from_thread_handle (self, 
> handle)

Spurious whitespace at the end of this line.

> +                        ->  gdb.InferiorThread.  */
> +
> +PyObject *
> +gdbpy_thread_from_thread_handle (PyObject *self, PyObject *args, 
> PyObject *kw)
> +{
> +  PyObject *handle_obj, *result;
> +  inferior_object *inf_obj = (inferior_object *) self;
> +  static const char *keywords[] = { "thread_handle", NULL };
> +
> +  INFPY_REQUIRE_VALID (inf_obj);
> +
> +  if (! gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, 
> &handle_obj))
> +    return NULL;
> +
> +  result = Py_None;
> +
> +  if (gdbpy_is_value_object (handle_obj))
> +    {
> +      TRY
> +	{
> +	  struct thread_info *thread_info;
> +	  struct value *val = value_object_to_value (handle_obj);
> +
> +	  thread_info = find_thread_by_handle (val, inf_obj->inferior);
> +	  if (thread_info != NULL)
> +	    {
> +	      result = (PyObject *) find_thread_object (thread_info->ptid);
> +	      if (result != NULL)
> +		Py_INCREF (result);
> +	    }
> +	}
> +      CATCH (except, RETURN_MASK_ALL)
> +	{
> +	  GDB_PY_HANDLE_EXCEPTION (except);
> +	}
> +      END_CATCH
> +    }

Should we raise a TypeError if the argument has the wrong type?  As a 
user not understanding why I always receive None because I made a silly 
typo, I would like if GDB told me I'm passing the wrong thing.

TypeError seems to be exactly for that:
https://docs.python.org/3/library/exceptions.html#TypeError

> +
> +  return result;
> +}
> +
> +
>  static void
>  infpy_dealloc (PyObject *obj)
>  {
> @@ -861,6 +904,10 @@ Write the given buffer object to the inferior's 
> memory." },
>      METH_VARARGS | METH_KEYWORDS,
>      "search_memory (address, length, pattern) -> long\n\
>  Return a long with the address of a match, or None." },
> +  { "thread_from_thread_handle", (PyCFunction) 
> gdbpy_thread_from_thread_handle,

Should the function now be named infpy_ to match the current style?

> +     METH_VARARGS | METH_KEYWORDS,
> +     "thread_from_thread_handle (handle) -> gdb.InferiorThread.\n\

These last two lines should be indented with one less space.

> +Return thread object corresponding to thread handle." },
>    { NULL }
>  };
>
> diff --git a/gdb/python/python-internal.h 
> b/gdb/python/python-internal.h
> index ebb83f0..40e64f0 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -379,6 +379,8 @@ extern PyTypeObject breakpoint_object_type
>      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("breakpoint_object");
>  extern PyTypeObject frame_object_type
>      CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("frame_object");
> +extern PyTypeObject thread_object_type
> +    CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("thread_object");
>
>  typedef struct gdbpy_breakpoint_object
>  {

Thanks,

Simon