Re: [PATCH v3 0/4] Python safety initial work

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On 7/19/26 7:16 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> Tom> I rewrote my earlier Python safety series to use methods a while back.
> Tom> And while I like the result, I got bogged down converting things, so I
> Tom> thought I'd send some initial work, with the idea that if this looks
> Tom> good, other code can be transformed in pieces.
> 
> Tom> Ping.
> 
> Ping again - any thoughts on this?
> 
> Tom

I applied the series and tried to use it.

I found this code:
...
       /* Note this returns a borrowed reference.  */
       PyObject *arg = PyTuple_GetItem (args, i);
...
and decided to try to convert all PyTuple_GetItem calls.  The result of 
that exercise is attached.

I ended up also touching the wrapper function:
...
  static inline gdbpy_borrowed_ref<>
  gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
  {
-  PyObject *result = PyTuple_GetItem (tuple, pos);
+  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
    if (result == nullptr)
      throw gdb_python_exception ();
-  return result;
+  return (PyObject *)result;
  }
...
but perhaps you left that out intentionally?

Anyway, having done this exercise, I like the idea of making borrowed 
refs explicit in a type.

I haven't used the gdbpy_tuple_get_item wrapper, AFAICT it was not 
applicable.

Acked-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-python-Use-gdbpy_borrowed_ref-for-PyTuple_GetIte.patch --]
[-- Type: text/x-patch, Size: 4186 bytes --]

From c14d61964e1288951ab2bf020d7d75de7b33aaa0 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 21 Jul 2026 12:44:12 +0200
Subject: [PATCH] [gdb/python] Use gdbpy_borrowed_ref for PyTuple_GetItem

Use gdbpy_borrowed_ref and gdbpy_opt_borrowed_ref as return type of
PyTuple_GetItem.
---
 gdb/python/py-color.c    |  2 +-
 gdb/python/py-mi.c       |  3 +--
 gdb/python/py-unwind.c   | 10 +++++-----
 gdb/python/py-value.c    |  2 +-
 gdb/python/py-wrappers.h |  4 ++--
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 29a42977adc..62360333f8b 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -231,7 +231,7 @@ colorpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 	  uint8_t rgb[3];
 	  for (int i = 0; i < 3; ++i)
 	    {
-	      PyObject *item = PyTuple_GetItem (value_obj, i);
+	      gdbpy_borrowed_ref<> item = PyTuple_GetItem (value_obj, i);
 	      if (!PyLong_Check (item))
 		error (_("Item %d of an RGB tuple must be integer."), i);
 	      long item_value = -1;
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 73daefecb20..5f14cefb134 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -152,8 +152,7 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
 
   for (Py_ssize_t i = 0; i < n_args; ++i)
     {
-      /* Note this returns a borrowed reference.  */
-      PyObject *arg = PyTuple_GetItem (args, i);
+      gdbpy_opt_borrowed_ref<> arg = PyTuple_GetItem (args, i);
       if (arg == nullptr)
 	return nullptr;
       gdb::unique_xmalloc_ptr<char> str = python_string_to_host_string (arg);
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 13eced4cecb..279ee922d4e 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -918,8 +918,8 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
 
   if (pyuw_debug)
     {
-      PyObject *pyo_unwinder_name = PyTuple_GetItem (pyo_execute_ret.get (), 1);
-      gdb_assert (pyo_unwinder_name != nullptr);
+      gdbpy_borrowed_ref<> pyo_unwinder_name
+	= PyTuple_GetItem (pyo_execute_ret.get (), 1);
       gdb::unique_xmalloc_ptr<char> name
 	= python_string_to_host_string (pyo_unwinder_name);
 
@@ -935,15 +935,15 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
     }
 
   /* Received UnwindInfo, cache data.  */
-  PyObject *pyo_unwind_info = PyTuple_GetItem (pyo_execute_ret.get (), 0);
-  gdb_assert (pyo_unwind_info != nullptr);
+  gdbpy_borrowed_ref<> pyo_unwind_info
+    = PyTuple_GetItem (pyo_execute_ret.get (), 0);
   if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type))
     error (_("an Unwinder should return gdb.UnwindInfo, not %s."),
 	   gdbpy_py_obj_tp_name (pyo_unwind_info).c_str ());
 
   {
     unwind_info_object *unwind_info =
-      (unwind_info_object *) pyo_unwind_info;
+      (unwind_info_object *) (PyObject *)pyo_unwind_info;
     int reg_count = unwind_info->saved_regs->size ();
 
     cached_frame
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 5b38110396e..0525f4b8bc9 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1208,7 +1208,7 @@ valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
       vargs = XALLOCAVEC (struct value *, args_count);
       for (i = 0; i < args_count; i++)
 	{
-	  PyObject *item = PyTuple_GetItem (args, i);
+	  gdbpy_opt_borrowed_ref<> item = PyTuple_GetItem (args, i);
 
 	  if (item == NULL)
 	    return NULL;
diff --git a/gdb/python/py-wrappers.h b/gdb/python/py-wrappers.h
index 6c2b5e4d41e..be0a0e31ef5 100644
--- a/gdb/python/py-wrappers.h
+++ b/gdb/python/py-wrappers.h
@@ -285,10 +285,10 @@ gdbpy_tuple_new (Py_ssize_t len)
 static inline gdbpy_borrowed_ref<>
 gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
 {
-  PyObject *result = PyTuple_GetItem (tuple, pos);
+  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
   if (result == nullptr)
     throw gdb_python_exception ();
-  return result;
+  return (PyObject *)result;
 }
 
 /* Wrapper for PyTuple_SetItem.  */

base-commit: 9036979ff84d387211315801a2b0611f7b716ea0
-- 
2.51.0