[3/4] gdb/python: convert Frame.read_register to take named arguments

Message ID aff9b4499f8dc1e575ab897377d23d7c91e2ca78.1680177890.git.aburgess@redhat.com
State New
Headers
Series Python API: Accept named arguments in a few more places |

Commit Message

Andrew Burgess March 30, 2023, 12:10 p.m. UTC
  Following on from the previous commit, this updates
Frame.read_register to accept named arguments.  As with the previous
commit there's no huge benefit for the users in accepting named
arguments here -- this function only takes a single argument after
all.

But I do think it is worth keeping Frame.read_register method in sync
with the PendingFrame.read_register method, this allows for the
possibility that the user has some code that can operate on either a
Frame or a Pending frame.

Minor update to allow for named arguments, and an extra test to check
the new functionality.
---
 gdb/doc/python.texi                   |  2 +-
 gdb/python/py-frame.c                 | 11 +++++++----
 gdb/testsuite/gdb.python/py-frame.exp |  6 ++++++
 3 files changed, 14 insertions(+), 5 deletions(-)
  

Comments

Eli Zaretskii March 30, 2023, 2:10 p.m. UTC | #1
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Thu, 30 Mar 2023 13:10:22 +0100
> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 4f62ca7d95e..f54909cc05d 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -5380,7 +5380,7 @@
>  @end defun
>  
>  @anchor{gdbpy_frame_read_register}
> -@defun Frame.read_register (register)
> +@defun Frame.read_register (@var{register})

Same comment here: I don't understand why you need @var in the
argument list of a @defun.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4f62ca7d95e..f54909cc05d 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5380,7 +5380,7 @@ 
 @end defun
 
 @anchor{gdbpy_frame_read_register}
-@defun Frame.read_register (register)
+@defun Frame.read_register (@var{register})
 Return the value of @var{register} in this frame.  Returns a
 @code{Gdb.Value} object.  Throws an exception if @var{register} does
 not exist.  The @var{register} argument must be one of the following:
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 00cd4bee492..3a033ac7570 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -238,13 +238,15 @@  frapy_pc (PyObject *self, PyObject *args)
    Returns the value of a register in this frame.  */
 
 static PyObject *
-frapy_read_register (PyObject *self, PyObject *args)
+frapy_read_register (PyObject *self, PyObject *args, PyObject *kw)
 {
   PyObject *pyo_reg_id;
   PyObject *result = nullptr;
 
-  if (!PyArg_UnpackTuple (args, "read_register", 1, 1, &pyo_reg_id))
-    return NULL;
+  static const char *keywords[] = { "register", nullptr };
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &pyo_reg_id))
+    return nullptr;
+
   try
     {
       scoped_value_mark free_values;
@@ -766,7 +768,8 @@  Return the reason why it's not possible to find frames older than this." },
   { "pc", frapy_pc, METH_NOARGS,
     "pc () -> Long.\n\
 Return the frame's resume address." },
-  { "read_register", frapy_read_register, METH_VARARGS,
+  { "read_register", (PyCFunction) frapy_read_register,
+    METH_VARARGS | METH_KEYWORDS,
     "read_register (register_name) -> gdb.Value\n\
 Return the value of the register in the frame." },
   { "block", frapy_block, METH_NOARGS,
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 5aebb6beb5f..07997326d09 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -115,6 +115,12 @@  gdb_test "python print ('result = %s' % (f0.read_register('pc') == f0.pc()))" \
   " = True" \
   "test Frame.read_register(pc)"
 
+# Repeat the previous test, but this time use named arguments for the
+# read_register method call.
+gdb_test "python print ('result = %s' % (f0.read_register(register = 'pc') == f0.pc()))" \
+  " = True" \
+  "test Frame.read_register() using named arguments"
+
 # Test arch-specific register name.
 set pc ""
 if {[is_amd64_regs_target]} {