Re: [PATCH 1/2] gdb: Add a find method for RegisterDescriptorIterator

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Eli Zaretskii <eliz@gnu.org> [2020-07-24 13:32:17 +0300]:

> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Date: Fri, 24 Jul 2020 10:57:51 +0100
> > 
> > +It is also possible to lookup a register descriptor based on its name
> > +using the following @code{gdb.RegisterDescriptorIterator} function:
> > +
> > +@defun RegisterDescriptorIterator.find ( @var{name} )
>                                            ^          ^
> Please remove these extra blanks, they are against our coding
> conventions.
> 
> > +Returns a @code{gdb.RegisterDescriptor} for the register with name
> > +@var{name}, or @code{None} if there is no register with that name.
> > +@end defun
> 
> Is it necessary or useful to say what kind of data type is NAME?  Or
> will this be obvious to any Python programmer?

Thanks for the feedback.  The patch below includes updated
documentation.  How's this?

Thanks,
Andrew

--

commit efe6a55ba25e088c67cde777bc8d524281356bb2
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Jul 22 14:02:30 2020 +0100

    gdb: Add a find method for RegisterDescriptorIterator
    
    Adds a new method 'find' to the gdb.RegisterDescriptorIterator class,
    this allows gdb.RegisterDescriptor objects to be looked up directly by
    register name rather than having to iterate over all registers.
    
    This will be of use for a later commit.
    
    I've documented the new function in the manual, but I don't think a
    NEWS entry is required here, as, since the last release, the whole
    register descriptor mechanism is new, and is already mentioned in the
    NEWS file.
    
    gdb/ChangeLog:
    
            * python/py-registers.c: Add 'user-regs.h' include.
            (register_descriptor_iter_find): New function.
            (register_descriptor_iterator_object_methods): New static global
            methods array.
            (register_descriptor_iterator_object_type): Add pointer to methods
            array.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.python/py-arch-reg-names.exp: Add additional tests.
    
    gdb/doc/ChangeLog:
    
            * python.texi (Registers In Python): Document new find function.

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 4fb994ca6d9..c9dc1ff3b71 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5755,6 +5755,15 @@
 The name of this register.
 @end defvar
 
+It is also possible to lookup a register descriptor based on its name
+using the following @code{gdb.RegisterDescriptorIterator} function:
+
+@defun RegisterDescriptorIterator.find (@var{name})
+Takes @var{name} as an argument, which must be a string, and returns a
+@code{gdb.RegisterDescriptor} for the register with that name, or
+@code{None} if there is no register with that name.
+@end defun
+
 Python code can also request from a @code{gdb.Architecture}
 information about the set of register groups available on a given
 architecture
diff --git a/gdb/python/py-registers.c b/gdb/python/py-registers.c
index f64ca3c401b..fffe3ecb1e6 100644
--- a/gdb/python/py-registers.c
+++ b/gdb/python/py-registers.c
@@ -23,6 +23,7 @@
 #include "disasm.h"
 #include "reggroups.h"
 #include "python-internal.h"
+#include "user-regs.h"
 #include <unordered_map>
 
 /* Token to access per-gdbarch data related to register descriptors.  */
@@ -337,6 +338,38 @@ gdbpy_register_descriptor_iter_next (PyObject *self)
   while (true);
 }
 
+/* Implement:
+
+   gdb.RegisterDescriptorIterator.find (self, name) -> gdb.RegisterDescriptor
+
+   Look up a descriptor for register with NAME.  If no matching register is
+   found then return None.  */
+
+static PyObject *
+register_descriptor_iter_find (PyObject *self, PyObject *args, PyObject *kw)
+{
+  static const char *keywords[] = { "name", NULL };
+  const char *register_name = NULL;
+
+  register_descriptor_iterator_object *iter_obj
+    = (register_descriptor_iterator_object *) self;
+  struct gdbarch *gdbarch = iter_obj->gdbarch;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s", keywords,
+					&register_name))
+    return NULL;
+
+  if (register_name != NULL && *register_name != '\0')
+    {
+      int regnum = user_reg_map_name_to_regnum (gdbarch, register_name,
+						strlen (register_name));
+      if (regnum >= 0)
+	return gdbpy_get_register_descriptor (gdbarch, regnum).release ();
+    }
+
+  Py_RETURN_NONE;
+}
+
 /* Initializes the new Python classes from this file in the gdb module.  */
 
 int
@@ -377,6 +410,15 @@ gdbpy_initialize_registers ()
 	   (PyObject *) &register_descriptor_iterator_object_type));
 }
 
+static PyMethodDef register_descriptor_iterator_object_methods [] = {
+  { "find", (PyCFunction) register_descriptor_iter_find,
+    METH_VARARGS | METH_KEYWORDS,
+    "registers (name) -> gdb.RegisterDescriptor.\n\
+Return a register descriptor for the register NAME, or None if no register\n\
+with that name exists in this iterator." },
+  {NULL}  /* Sentinel */
+};
+
 PyTypeObject register_descriptor_iterator_object_type = {
   PyVarObject_HEAD_INIT (NULL, 0)
   "gdb.RegisterDescriptorIterator",	  	/*tp_name*/
@@ -405,7 +447,7 @@ PyTypeObject register_descriptor_iterator_object_type = {
   0,				  /*tp_weaklistoffset */
   gdbpy_register_descriptor_iter,	  /*tp_iter */
   gdbpy_register_descriptor_iter_next,  /*tp_iternext */
-  0				  /*tp_methods */
+  register_descriptor_iterator_object_methods		/*tp_methods */
 };
 
 static gdb_PyGetSetDef gdbpy_register_descriptor_getset[] = {
diff --git a/gdb/testsuite/gdb.python/py-arch-reg-names.exp b/gdb/testsuite/gdb.python/py-arch-reg-names.exp
index 8dd34ef5fd2..891da1b6af5 100644
--- a/gdb/testsuite/gdb.python/py-arch-reg-names.exp
+++ b/gdb/testsuite/gdb.python/py-arch-reg-names.exp
@@ -104,3 +104,18 @@ gdb_py_test_silent_cmd \
 	 "    raise gdb.GdbError (\"miss-matched objects\")" \
 	 "\004" ] \
     "check names and objects match" 1
+
+# Ensure that the '.find' method on the iterator returns the same
+# Python object as we got from the iterator's list of descriptors.
+gdb_py_test_silent_cmd \
+    [multi_line \
+	 "python" \
+	 "def check_regs (arch, regs):" \
+	 "   for r in (regs):" \
+	 "     if (arch.registers ().find (r.name) != r):" \
+	 "       raise gdb.GdbError (\"object miss-match\")" \
+	 "end" ] \
+    "build check_obj function" 0
+gdb_py_test_silent_cmd \
+    "python check_regs (arch, arch.registers (\"general\"))" \
+    "ensure find gets expected descriptors" 1