[PATCH 4/5] Rewrite breakpoint location getter methods

[Tom Tromey <tom@tromey.com>]

This rewrites the breakpoint location getter methods to use the Python
safety API.
---
 gdb/python/py-breakpoint.c | 231 +++++++++++++++++++++------------------------
 1 file changed, 110 insertions(+), 121 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 40d1244abcf..b39093ebd81 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -67,6 +67,52 @@ struct gdbpy_breakpoint_location_object : public PyObject
   /* An owning reference to the location's breakpoint owner.  */
   gdbpy_breakpoint_object *owner;
 
+  /* Python function to test whether or not the breakpoint location is
+     enabled.  */
+  bool get_enabled () const
+  {
+    require_valid ();
+    return bp_loc->enabled;
+  }
+
+  /* Python function to get owner of breakpoint location, which is of
+     type gdb.Breakpoint.  */
+  gdbpy_ref<> get_owner () const
+  {
+    require_valid ();
+    return gdbpy_ref<>::new_reference (owner);
+  }
+
+  /* Python function to get address of breakpoint location.  */
+  CORE_ADDR get_address () const
+  {
+    require_valid ();
+    return bp_loc->address;
+  }
+
+  /* Python function to get the function name of where this location
+     was set.  */
+  const char *get_function () const
+  {
+    require_valid ();
+    return bp_loc->function_name.get ();
+  }
+
+  /* Return the full filename of the location.  */
+  const char *get_fullname () const
+  {
+    require_valid ();
+    if (bp_loc->symtab == nullptr)
+      return nullptr;
+    return bp_loc->symtab->fullname ();
+  }
+
+  /* Accessor to get the source location.  */
+  gdbpy_ref<> get_source_location () const;
+
+  /* Accessor to get the thread groups.  */
+  gdbpy_ref<> get_thread_groups () const;
+
   /* Implementation of 'repr'.  */
   gdbpy_ref<> repr ();
 
@@ -82,15 +128,6 @@ struct gdbpy_breakpoint_location_object : public PyObject
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_location_object>);
 
-/* Require that BREAKPOINT and LOCATION->OWNER are the same; throw a Python
-   exception if they are not.  */
-#define BPLOCPY_REQUIRE_VALID(Breakpoint, Location)                         \
-    do {                                                                    \
-      if ((Breakpoint)->bp != (Location)->bp_loc->owner)                    \
-	return PyErr_Format (PyExc_RuntimeError,                            \
-			     _("Breakpoint location is invalid."));	    \
-    } while (0)
-
 /* Require that BREAKPOINT and LOCATION->OWNER are the same; throw a Python
    exception if they are not.  This macro is for use in setter functions.  */
 #define BPLOCPY_SET_REQUIRE_VALID(Breakpoint, Location)                     \
@@ -1604,45 +1641,6 @@ bplocpy_set_enabled (PyObject *py_self, PyObject *newvalue, void *closure)
   return 0;
 }
 
-/* Python function to test whether or not the breakpoint location is enabled.  */
-
-static PyObject *
-bplocpy_get_enabled (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-
-  if (self->bp_loc->enabled)
-    return py_true ().release ();
-  else
-    return py_false ().release ();
-}
-
-/* Python function to get address of breakpoint location.  */
-
-static PyObject *
-bplocpy_get_address (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  return gdb_py_object_from_ulongest (self->bp_loc->address).release ();
-}
-
-/* Python function to get owner of breakpoint location, which
-   is of type gdb.Breakpoint.  */
-
-static PyObject *
-bplocpy_get_owner (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  Py_INCREF (self->owner);
-  return self->owner;
-}
-
 /* Attempt to get fully resolved file path for symtab.  */
 
 static gdbpy_ref<>
@@ -1650,99 +1648,69 @@ bploc_filepath (struct symtab *bploc_symtab)
 {
   /* The exception is not ours to handle.  We should always
      return some string value and filename is never null.  */
+  const char *filename = nullptr;
   try
     {
-      const char *full = symtab_to_fullname (bploc_symtab);
-      if (full)
-	return host_string_to_python_string (full);
+      filename = symtab_to_fullname (bploc_symtab);
     }
   catch (const gdb_exception &except)
     {
     }
 
-  return host_string_to_python_string (bploc_symtab->filename ());
+  if (filename == nullptr)
+    filename = bploc_symtab->filename ();
+  gdb_assert (filename != nullptr);
+
+  gdbpy_ref<> result = host_string_to_python_string (filename);
+  /* FIXME: Python safety: host_string_to_python_string should
+     throw.  */
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
 }
 
 /* Python function to get the source file name path and line number
    where this breakpoint location was set.  */
 
-static PyObject *
-bplocpy_get_source_location (PyObject *py_self, void *closure)
+gdbpy_ref<>
+gdbpy_breakpoint_location_object::get_source_location () const
 {
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  if (self->bp_loc->symtab)
+  require_valid ();
+  if (bp_loc->symtab != nullptr)
     {
-      gdbpy_ref<> tup (PyTuple_New (2));
-      if (tup == nullptr)
-	return nullptr;
-      gdbpy_ref<> filename = bploc_filepath (self->bp_loc->symtab);
-      if (filename == nullptr)
-	return nullptr;
-      auto line = gdb_py_object_from_ulongest (self->bp_loc->line_number);
+      gdbpy_ref<> tup = gdbpy_tuple_new (2);
+      gdbpy_ref<> filename = bploc_filepath (bp_loc->symtab);
+      auto line = gdb_py_object_from_ulongest (bp_loc->line_number);
+      /* FIXME: Python safety: gdb_py_object_from_ulongest should
+	 throw.  */
       if (line == nullptr)
-	return nullptr;
-      if (PyTuple_SetItem (tup.get (), 0, filename.release ()) == -1
-	  || PyTuple_SetItem (tup.get (), 1, line.release ()) == -1)
-	return nullptr;
-      return tup.release ();
+	throw gdb_python_exception ();
+      gdbpy_tuple_set_item (tup, 0, std::move (filename));
+      gdbpy_tuple_set_item (tup, 1, std::move (line));
+      return tup;
     }
-  else
-    return py_none ().release ();
-}
 
-/* Python function to get the function name of where this location was set.  */
-
-static PyObject *
-bplocpy_get_function (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  const auto fn_name = self->bp_loc->function_name.get ();
-  if (fn_name != nullptr)
-    return host_string_to_python_string (fn_name).release ();
-  return py_none ().release ();
+  return py_none ();
 }
 
-static PyObject *
-bplocpy_get_thread_groups (PyObject *py_self, void *closure)
+gdbpy_ref<>
+gdbpy_breakpoint_location_object::get_thread_groups () const
 {
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  gdbpy_ref<> list (PyList_New (0));
-  if (list == nullptr)
-    return nullptr;
+  require_valid ();
+  gdbpy_ref<> list = gdbpy_list_new (0);
   for (inferior *inf : all_inferiors ())
     {
-      if (inf->pspace == self->bp_loc->pspace)
+      if (inf->pspace == bp_loc->pspace)
 	{
 	  gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);
+	  /* FIXME: Python safety: gdb_py_object_from_ulongest should
+	     throw.  */
 	  if (num == nullptr)
-	    return nullptr;
-	  if (PyList_Append (list.get (), num.get ()) != 0)
-	    return nullptr;
+	    throw gdb_python_exception ();
+	  gdbpy_list_append (list, num);
 	}
     }
-  return list.release ();
-}
-
-static PyObject *
-bplocpy_get_fullname (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  const auto symtab = self->bp_loc->symtab;
-  if (symtab != nullptr && symtab->fullname () != nullptr)
-    {
-      gdbpy_ref<> fullname
-	= host_string_to_python_string (symtab->fullname ());
-      return fullname.release ();
-    }
-  return py_none ().release ();
+  return list;
 }
 
 /* De-allocation function to be called for the Python object.  */
@@ -1798,19 +1766,40 @@ gdbpy_breakpoint_location_object::repr ()
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
-  { "enabled", bplocpy_get_enabled, bplocpy_set_enabled,
+  { "enabled",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_enabled>,
+    bplocpy_set_enabled,
     "Boolean telling whether the breakpoint is enabled.", NULL },
-  { "owner", bplocpy_get_owner, NULL,
+  { "owner",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_owner>,
+    nullptr,
     "Get the breakpoint owner object", NULL },
-  { "address", bplocpy_get_address, NULL,
+  { "address",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_address>,
+    nullptr,
     "Get address of where this location was set", NULL},
-  { "source", bplocpy_get_source_location, NULL,
+  { "source",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_source_location>,
+    nullptr,
     "Get file and line number of where this location was set", NULL},
-  { "function", bplocpy_get_function, NULL,
+  { "function",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_function>,
+    nullptr,
     "Get function of where this location was set", NULL },
-  { "fullname", bplocpy_get_fullname, NULL,
+  { "fullname",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_fullname>,
+    nullptr,
     "Get fullname of where this location was set", NULL },
-  { "thread_groups", bplocpy_get_thread_groups, NULL,
+  { "thread_groups",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_thread_groups>,
+    nullptr,
     "Get thread groups where this location is in", NULL },
   { NULL }  /* Sentinel.  */
 };

-- 
2.49.0