[1/4] Change pspace_to_pspace_object to return a new reference

Message ID 20180913053007.11780-2-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 13, 2018, 5:30 a.m. UTC
  This changes pspace_to_pspace_object to return a new reference and
fixes up all the callers.

gdb/ChangeLog
2018-09-12  Tom Tromey  <tom@tromey.com>

	* python/python-internal.h (pspace_to_pspace_object): Change
	return type.
	* python/py-newobjfileevent.c
	(create_clear_objfiles_event_object): Update.
	* python/py-xmethods.c (gdbpy_get_matching_xmethod_workers):
	Update.
	* python/python.c (gdbpy_get_current_progspace): Update.
	(gdbpy_progspaces): Update.
	* python/py-progspace.c (pspace_to_pspace_object): Return a new
	reference.
	* python/py-objfile.c (objfpy_get_progspace): Update.
	* python/py-prettyprint.c (find_pretty_printer_from_progspace):
	Update.
---
 gdb/ChangeLog                   | 16 ++++++++++++++++
 gdb/python/py-newobjfileevent.c | 10 ++++------
 gdb/python/py-objfile.c         |  6 ++----
 gdb/python/py-prettyprint.c     |  6 +++---
 gdb/python/py-progspace.c       | 31 ++++++++++++++++---------------
 gdb/python/py-xmethods.c        |  6 +++---
 gdb/python/python-internal.h    |  3 +--
 gdb/python/python.c             | 11 +++--------
 8 files changed, 48 insertions(+), 41 deletions(-)
  

Comments

Simon Marchi Sept. 16, 2018, 12:57 a.m. UTC | #1
On 2018-09-13 1:30 a.m., Tom Tromey wrote:
> diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
> index c2b40ff5352..8fd160c7bf8 100644
> --- a/gdb/python/py-objfile.c
> +++ b/gdb/python/py-objfile.c
> @@ -167,10 +167,8 @@ objfpy_get_progspace (PyObject *self, void *closure)
>  
>    if (obj->objfile)
>      {
> -      PyObject *pspace =  pspace_to_pspace_object (obj->objfile->pspace);
> -
> -      Py_XINCREF (pspace);
> -      return pspace;
> +      gdbpy_ref<> pspace = pspace_to_pspace_object (obj->objfile->pspace);
> +      return pspace.get ();
>      }

Can you double check this, shouldn't it be .release()?  Otherwise, LGTM.

Simon
  
Simon Marchi Sept. 16, 2018, 1:19 a.m. UTC | #2
On 2018-09-13 1:30 a.m., Tom Tromey wrote:
> This changes pspace_to_pspace_object to return a new reference and
> fixes up all the callers.

Actually, I think the patch is missing a hunk or something, I get this error:

/home/simark/src/binutils-gdb/gdb/python/py-inferior.c:474:13: error: no viable conversion from 'gdbpy_ref<>' (aka 'gdb::ref_ptr<_object, gdbpy_ref_policy<_object> >') to 'PyObject *' (aka '_object *')
  PyObject *py_pspace = pspace_to_pspace_object (pspace);
            ^           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.


Simon
  
Simon Marchi Sept. 16, 2018, 1:57 a.m. UTC | #3
On 2018-09-15 9:19 p.m., Simon Marchi wrote:
> On 2018-09-13 1:30 a.m., Tom Tromey wrote:
>> This changes pspace_to_pspace_object to return a new reference and
>> fixes up all the callers.
> 
> Actually, I think the patch is missing a hunk or something, I get this error:
> 
> /home/simark/src/binutils-gdb/gdb/python/py-inferior.c:474:13: error: no viable conversion from 'gdbpy_ref<>' (aka 'gdb::ref_ptr<_object, gdbpy_ref_policy<_object> >') to 'PyObject *' (aka '_object *')
>   PyObject *py_pspace = pspace_to_pspace_object (pspace);
>             ^           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.

Ah ok, this is the code I added recently, that's why.  I think changing it to

  return pspace_to_pspace_object (pspace).release ();

should work.

Simon
  
Tom Tromey Sept. 16, 2018, 12:59 p.m. UTC | #4
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +      gdbpy_ref<> pspace = pspace_to_pspace_object (obj->objfile->pspace);
>> +      return pspace.get ();
>> }

Simon> Can you double check this, shouldn't it be .release()?  Otherwise, LGTM.

Yes it should.  Whoops :(
I made this change.

Tom
  
Tom Tromey Sept. 16, 2018, 12:59 p.m. UTC | #5
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Ah ok, this is the code I added recently, that's why.  I think changing it to
Simon>   return pspace_to_pspace_object (pspace).release ();
Simon> should work.

Yep, I got this on rebasing and did what you suggest here.

Tom
  

Patch

diff --git a/gdb/python/py-newobjfileevent.c b/gdb/python/py-newobjfileevent.c
index a9341a3be18..aa31fb4849b 100644
--- a/gdb/python/py-newobjfileevent.c
+++ b/gdb/python/py-newobjfileevent.c
@@ -66,12 +66,10 @@  create_clear_objfiles_event_object (void)
   if (objfile_event == NULL)
     return NULL;
 
-  /* Note that pspace_to_pspace_object returns a borrowed reference,
-     so we don't need a decref here.  */
-  PyObject *py_progspace = pspace_to_pspace_object (current_program_space);
-  if (!py_progspace || evpy_add_attribute (objfile_event.get (),
-					   "progspace",
-					   py_progspace) < 0)
+  gdbpy_ref<> py_progspace = pspace_to_pspace_object (current_program_space);
+  if (py_progspace == NULL || evpy_add_attribute (objfile_event.get (),
+						  "progspace",
+						  py_progspace.get ()) < 0)
     return NULL;
 
   return objfile_event;
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index c2b40ff5352..8fd160c7bf8 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -167,10 +167,8 @@  objfpy_get_progspace (PyObject *self, void *closure)
 
   if (obj->objfile)
     {
-      PyObject *pspace =  pspace_to_pspace_object (obj->objfile->pspace);
-
-      Py_XINCREF (pspace);
-      return pspace;
+      gdbpy_ref<> pspace = pspace_to_pspace_object (obj->objfile->pspace);
+      return pspace.get ();
     }
 
   Py_RETURN_NONE;
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 7da0f2d9af2..9aadd3b27c9 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -128,11 +128,11 @@  find_pretty_printer_from_objfiles (PyObject *value)
 static PyObject *
 find_pretty_printer_from_progspace (PyObject *value)
 {
-  PyObject *obj = pspace_to_pspace_object (current_program_space);
+  gdbpy_ref<> obj = pspace_to_pspace_object (current_program_space);
 
-  if (!obj)
+  if (obj == NULL)
     return NULL;
-  gdbpy_ref<> pp_list (pspy_get_printers (obj, NULL));
+  gdbpy_ref<> pp_list (pspy_get_printers (obj.get (), NULL));
   return search_pp_list (pp_list.get (), value);
 }
 
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 3eaa466666d..2aeeca92eb2 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -337,30 +337,31 @@  py_free_pspace (struct program_space *pspace, void *datum)
   object->pspace = NULL;
 }
 
-/* Return a borrowed reference to the Python object of type Pspace
+/* Return a new reference to the Python object of type Pspace
    representing PSPACE.  If the object has already been created,
    return it.  Otherwise, create it.  Return NULL and set the Python
    error on failure.  */
 
-PyObject *
+gdbpy_ref<>
 pspace_to_pspace_object (struct program_space *pspace)
 {
-  gdbpy_ref<pspace_object> object
-    ((pspace_object *) program_space_data (pspace, pspy_pspace_data_key));
-  if (object == NULL)
+  PyObject *result
+    ((PyObject *) program_space_data (pspace, pspy_pspace_data_key));
+  if (result == NULL)
     {
-      object.reset (PyObject_New (pspace_object, &pspace_object_type));
-      if (object != NULL)
-	{
-	  if (!pspy_initialize (object.get ()))
-	    return NULL;
-
-	  object->pspace = pspace;
-	  set_program_space_data (pspace, pspy_pspace_data_key, object.get ());
-	}
+      gdbpy_ref<pspace_object> object
+	((pspace_object *) PyObject_New (pspace_object, &pspace_object_type));
+      if (object == NULL)
+	return NULL;
+      if (!pspy_initialize (object.get ()))
+	return NULL;
+
+      object->pspace = pspace;
+      set_program_space_data (pspace, pspy_pspace_data_key, object.get ());
+      result = (PyObject *) object.release ();
     }
 
-  return (PyObject *) object.release ();
+  return gdbpy_ref<>::new_reference (result);
 }
 
 int
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index f7e3c37210c..c568295a165 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -122,7 +122,6 @@  gdbpy_get_matching_xmethod_workers
    std::vector<xmethod_worker_up> *dm_vec)
 {
   struct objfile *objfile;
-  PyObject *py_progspace;
 
   gdb_assert (obj_type != NULL && method_name != NULL);
 
@@ -170,10 +169,11 @@  gdbpy_get_matching_xmethod_workers
 
   /* Gather debug methods matchers registered with the current program
      space.  */
-  py_progspace = pspace_to_pspace_object (current_program_space);
+  gdbpy_ref<> py_progspace = pspace_to_pspace_object (current_program_space);
   if (py_progspace != NULL)
     {
-      gdbpy_ref<> pspace_matchers (pspy_get_xmethods (py_progspace, NULL));
+      gdbpy_ref<> pspace_matchers (pspy_get_xmethods (py_progspace.get (),
+						      NULL));
 
       gdbpy_ref<> temp (PySequence_Concat (py_xmethod_matcher_list.get (),
 					   pspace_matchers.get ()));
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 3874fdc5e79..a6dc5f090b5 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -517,8 +517,7 @@  PyObject *value_to_value_object (struct value *v);
 PyObject *type_to_type_object (struct type *);
 PyObject *frame_info_to_frame_object (struct frame_info *frame);
 PyObject *symtab_to_linetable_object (PyObject *symtab);
-PyObject *pspace_to_pspace_object (struct program_space *)
-    CPYCHECKER_RETURNS_BORROWED_REF;
+gdbpy_ref<> pspace_to_pspace_object (struct program_space *);
 PyObject *pspy_get_printers (PyObject *, void *);
 PyObject *pspy_get_frame_filters (PyObject *, void *);
 PyObject *pspy_get_frame_unwinders (PyObject *, void *);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 6f798a0e461..93d6d1156d8 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1339,12 +1339,7 @@  gdbpy_print_stack (void)
 static PyObject *
 gdbpy_get_current_progspace (PyObject *unused1, PyObject *unused2)
 {
-  PyObject *result;
-
-  result = pspace_to_pspace_object (current_program_space);
-  if (result)
-    Py_INCREF (result);
-  return result;
+  return pspace_to_pspace_object (current_program_space).release ();
 }
 
 /* Return a sequence holding all the Progspaces.  */
@@ -1360,9 +1355,9 @@  gdbpy_progspaces (PyObject *unused1, PyObject *unused2)
 
   ALL_PSPACES (ps)
   {
-    PyObject *item = pspace_to_pspace_object (ps);
+    gdbpy_ref<> item = pspace_to_pspace_object (ps);
 
-    if (!item || PyList_Append (list.get (), item) == -1)
+    if (item == NULL || PyList_Append (list.get (), item.get ()) == -1)
       return NULL;
   }