public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod
@ 2024-06-11  8:55 Tom de Vries
  2024-06-11 19:55 ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2024-06-11  8:55 UTC (permalink / raw)
  To: gdb-patches

I noticed code like this:
...
      gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
                                              "i", num_to_scroll, nullptr));
...

The nullptr is superfluous, the format string already indicates that there's
only one argument.

OTOH, passing no args does use a nullptr:
...
      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
                                               nullptr));
...

Furthermore, choosing the right format string chars can be tricky.

Add non-variadic versions of PyObject_CallMethod that hide these details, such
that we can use the more intuitive:
...
      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get(), "hscroll",
                                                   num_to_scroll));
...
and:
...
      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "render"));
...

Tested on x86_64-linux.
---
 gdb/python/py-breakpoint.c       |  2 +-
 gdb/python/py-disasm.c           |  6 ++--
 gdb/python/py-finishbreakpoint.c |  4 +--
 gdb/python/py-framefilter.c      | 17 +++++----
 gdb/python/py-tui.c              | 20 +++++------
 gdb/python/python-internal.h     | 61 ++++++++++++++++++++++++++++++--
 6 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index da74d69d357..ce0096680f5 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1174,7 +1174,7 @@ gdbpy_breakpoint_cond_says_stop (const struct extension_language_defn *extlang,
 
   if (PyObject_HasAttrString (py_bp, stop_func))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (py_bp, stop_func, NULL));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (py_bp, stop_func));
 
       stop = 1;
       if (result != NULL)
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9337514acba..7fd595bf8c4 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -855,9 +855,9 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff,
 
   /* Now call the DisassembleInfo.read_memory method.  This might have been
      overridden by the user.  */
-  gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
-					       "read_memory",
-					       "I" GDB_PY_LL_ARG, len, offset));
+  gdbpy_ref<> result_obj (gdb_PyObject_CallMethod ((PyObject *) obj,
+						   "read_memory",
+						   len, offset));
 
   /* Handle any exceptions.  */
   if (result_obj == nullptr)
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index c74a2473a81..b6f85c202d6 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -344,8 +344,8 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
   if (bpfinish_obj->py_bp.bp->enable_state == bp_enabled
       && PyObject_HasAttrString (py_obj, outofscope_func))
     {
-      gdbpy_ref<> meth_result (PyObject_CallMethod (py_obj, outofscope_func,
-						    NULL));
+      gdbpy_ref<> meth_result (gdb_PyObject_CallMethod (py_obj,
+							outofscope_func));
       if (meth_result == NULL)
 	gdbpy_print_stack ();
     }
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 0cd15977d2f..6ae14c223b5 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -59,7 +59,7 @@ extract_sym (PyObject *obj, gdb::unique_xmalloc_ptr<char> *name,
 	     struct symbol **sym, const struct block **sym_block,
 	     const struct language_defn **language)
 {
-  gdbpy_ref<> result (PyObject_CallMethod (obj, "symbol", NULL));
+  gdbpy_ref<> result (gdb_PyObject_CallMethod (obj, "symbol"));
 
   if (result == NULL)
     return EXT_LANG_BT_ERROR;
@@ -130,7 +130,7 @@ extract_value (PyObject *obj, struct value **value)
 {
   if (PyObject_HasAttrString (obj, "value"))
     {
-      gdbpy_ref<> vresult (PyObject_CallMethod (obj, "value", NULL));
+      gdbpy_ref<> vresult (gdb_PyObject_CallMethod (obj, "value"));
 
       if (vresult == NULL)
 	return EXT_LANG_BT_ERROR;
@@ -264,7 +264,7 @@ get_py_iter_from_func (PyObject *filter, const char *func)
 {
   if (PyObject_HasAttrString (filter, func))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (filter, func, NULL));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (filter, func));
 
       if (result != NULL)
 	{
@@ -790,8 +790,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
   /* Get the underlying frame.  This is needed to determine GDB
   architecture, and also, in the cases of frame variables/arguments to
   read them if they returned filter object requires us to do so.  */
-  gdbpy_ref<> py_inf_frame (PyObject_CallMethod (filter, "inferior_frame",
-						 NULL));
+  gdbpy_ref<> py_inf_frame (gdb_PyObject_CallMethod (filter, "inferior_frame"));
   if (py_inf_frame == NULL)
     return EXT_LANG_BT_ERROR;
 
@@ -831,7 +830,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 	 address printing.  */
       if (PyObject_HasAttrString (filter, "address"))
 	{
-	  gdbpy_ref<> paddr (PyObject_CallMethod (filter, "address", NULL));
+	  gdbpy_ref<> paddr (gdb_PyObject_CallMethod (filter, "address"));
 
 	  if (paddr == NULL)
 	    return EXT_LANG_BT_ERROR;
@@ -906,7 +905,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
       /* Print frame function name.  */
       if (PyObject_HasAttrString (filter, "function"))
 	{
-	  gdbpy_ref<> py_func (PyObject_CallMethod (filter, "function", NULL));
+	  gdbpy_ref<> py_func (gdb_PyObject_CallMethod (filter, "function"));
 	  const char *function = NULL;
 
 	  if (py_func == NULL)
@@ -970,7 +969,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 
       if (PyObject_HasAttrString (filter, "filename"))
 	{
-	  gdbpy_ref<> py_fn (PyObject_CallMethod (filter, "filename", NULL));
+	  gdbpy_ref<> py_fn (gdb_PyObject_CallMethod (filter, "filename"));
 
 	  if (py_fn == NULL)
 	    return EXT_LANG_BT_ERROR;
@@ -994,7 +993,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 
       if (PyObject_HasAttrString (filter, "line"))
 	{
-	  gdbpy_ref<> py_line (PyObject_CallMethod (filter, "line", NULL));
+	  gdbpy_ref<> py_line (gdb_PyObject_CallMethod (filter, "line"));
 	  int line;
 
 	  if (py_line == NULL)
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 5dcec4bd2b1..57b13148e2e 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -164,8 +164,7 @@ tui_py_window::~tui_py_window ()
   if (m_window != nullptr
       && PyObject_HasAttrString (m_window.get (), "close"))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "close",
-					       nullptr));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "close"));
       if (result == nullptr)
 	gdbpy_print_stack ();
     }
@@ -198,8 +197,7 @@ tui_py_window::rerender ()
 
   if (PyObject_HasAttrString (m_window.get (), "render"))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "render",
-					       nullptr));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "render"));
       if (result == nullptr)
 	gdbpy_print_stack ();
     }
@@ -212,8 +210,8 @@ tui_py_window::do_scroll_horizontal (int num_to_scroll)
 
   if (PyObject_HasAttrString (m_window.get (), "hscroll"))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (m_window.get(), "hscroll",
-					       "i", num_to_scroll, nullptr));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "hscroll",
+						   num_to_scroll));
       if (result == nullptr)
 	gdbpy_print_stack ();
     }
@@ -226,8 +224,8 @@ tui_py_window::do_scroll_vertical (int num_to_scroll)
 
   if (PyObject_HasAttrString (m_window.get (), "vscroll"))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "vscroll",
-					       "i", num_to_scroll, nullptr));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "vscroll",
+						   num_to_scroll));
       if (result == nullptr)
 	gdbpy_print_stack ();
     }
@@ -248,9 +246,9 @@ tui_py_window::click (int mouse_x, int mouse_y, int mouse_button)
 
   if (PyObject_HasAttrString (m_window.get (), "click"))
     {
-      gdbpy_ref<> result (PyObject_CallMethod (m_window.get (), "click",
-					       "iii", mouse_x, mouse_y,
-					       mouse_button));
+      gdbpy_ref<> result (gdb_PyObject_CallMethod (m_window.get (), "click",
+						   mouse_x, mouse_y,
+						   mouse_button));
       if (result == nullptr)
 	gdbpy_print_stack ();
     }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index d07f239ca65..f4d7ae41c72 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -100,6 +100,12 @@
 #define PyEval_ReleaseLock()
 #endif
 
+/* Format string chars as defined here [1], in a readable and intuitive form.
+   [1] https://docs.python.org/3/c-api/arg.html#building-values.  */
+
+#define GDB_PY_INT_ARG "i"
+#define GDB_PY_UNSIGNED_INT_ARG "I"
+
 /* Python supplies HAVE_LONG_LONG and some `long long' support when it
    is available.  These defines let us handle the differences more
    cleanly.
@@ -154,8 +160,10 @@ typedef long Py_hash_t;
 
 template<typename... Args>
 static inline PyObject *
-gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
-			 Args... args) /* ARI: editCase function */
+gdb_compatibility_PyObject_CallMethod (PyObject *o, const char *method,
+				       const char *format,
+				       Args... args) /* ARI: editCase
+							function.  */
 {
   return PyObject_CallMethod (o,
 			      const_cast<char *> (method),
@@ -164,7 +172,54 @@ gdb_PyObject_CallMethod (PyObject *o, const char *method, const char *format,
 }
 
 #undef PyObject_CallMethod
-#define PyObject_CallMethod gdb_PyObject_CallMethod
+#define PyObject_CallMethod gdb_compatibility_PyObject_CallMethod
+
+/* Non-variadic versions of PyObject_CallMethod.  */
+
+static inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name)
+{
+  return PyObject_CallMethod (obj, name, nullptr);
+}
+
+template<typename T>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1)
+{
+  return PyObject_CallMethod (obj, name, GDB_PY_INT_ARG, arg1);
+}
+
+template<typename T, typename T2>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, unsigned int arg1,
+			 gdb_py_longest arg2)
+{
+  return PyObject_CallMethod (obj, name,
+			      GDB_PY_UNSIGNED_INT_ARG GDB_PY_LL_ARG,
+			      arg1, arg2);
+}
+
+template<typename T, typename T2, typename T3>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, T, T2, T3);
+
+template<>
+inline PyObject *
+gdb_PyObject_CallMethod (PyObject *obj, const char *name, int arg1, int arg2,
+			 int arg3)
+{
+  return PyObject_CallMethod (obj, name,
+			      GDB_PY_INT_ARG GDB_PY_INT_ARG GDB_PY_INT_ARG,
+			      arg1, arg2, arg3);
+}
 
 /* The 'name' parameter of PyErr_NewException was missing the 'const'
    qualifier in Python <= 3.4.  Hence, we wrap it in a function to

base-commit: 60e221e9b7fae5ff40b79b93bdbff909989c2bae
-- 
2.35.3


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-12  9:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-11  8:55 [PATCH v2] [gdb/python] Add non-variadic versions of PyObject_CallMethod Tom de Vries
2024-06-11 19:55 ` Tom Tromey
2024-06-11 22:31   ` Tom de Vries
2024-06-12  9:09     ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).