Re: [PATCH v2 2/4] Define gdb.Value(bufobj, type) constructor

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-02-19 16:41, Kevin Buettner wrote:
> Provided a buffer BUFOBJ and a type TYPE, construct a gdb.Value object
> with type TYPE, where the value's contents are taken from BUFOBJ.
> 
> E.g...
> 
> (gdb) python import struct
> (gdb) python unsigned_int_type=gdb.lookup_type('unsigned int')
> (gdb) python b=struct.pack('=I',0xdeadbeef)
> (gdb) python v=gdb.Value(b, unsigned_int_type) ; print("%#x" % v)
> 0xdeadbeef
> 
> This two argument form of the gdb.Value constructor may also be used
> to obtain gdb values from selected portions of buffers read with
> Inferior.read_memory().  The test case (which is in a separate patch)
> demonstrates this use case.
> 
> gdb/ChangeLog:
> 
> 	* python/py-value.c (convert_buffer_and_type_to_value): New
> 	function.
> 	(valpy_new): Parse arguments via gdb_PyArg_ParseTupleAndKeywords.
> 	Add support for handling an optional second argument.  Call
> 	convert_buffer_and_type_to_value as appropriate.
> ---
>  gdb/python/py-value.c | 72 
> ++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 62 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index 20ef5822f8..af5a75a4a4 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -107,22 +107,68 @@ note_value (value_object *value_obj)
>    values_in_python = value_obj;
>  }
> 
> +/* Convert a python object OBJ with type TYPE to a gdb value.  The
> +   python object in question must conform to the python buffer
> +   protocol.  On success, return the converted value, otherwise
> +   nullptr.  */
> +
> +static struct value *
> +convert_buffer_and_type_to_value (PyObject *obj, struct type *type)
> +{
> +  Py_buffer_up buffer_up;
> +  Py_buffer py_buf;
> +
> +  if (PyObject_CheckBuffer (obj)
> +      && PyObject_GetBuffer (obj, &py_buf, PyBUF_SIMPLE) == 0)
> +    {
> +      /* Got a buffer, py_buf, out of obj.  Cause it to be released
> +         when it goes out of scope.  */
> +      buffer_up.reset (&py_buf);
> +    }
> +  else
> +    {
> +      PyErr_SetString (PyExc_TypeError,
> +		       _("Object must support the python buffer protocol."));
> +      return nullptr;
> +    }
> +
> +  if (TYPE_LENGTH (type) > py_buf.len)
> +    {
> +      PyErr_SetString (PyExc_TypeError,
> +		       _("Size of type is larger than that of buffer object."));
> +      return nullptr;
> +    }

Another small thing I didn't spot when reading v1: I think it would be 
more appropriate to raise a ValueError in this last case.  TypeError 
would be if the arguments were of the wrong Python type, which is not 
the case here.  It just happens that the value we handle is a (GDB) 
type, but that's not the same kind of type.  If you do that change, 
don't forget to update the tests as well.

Otherwise, the whole series still LGTM.

Simon