From: Tom Tromey Date: Fri, 15 May 2026 17:42:08 +0000 (-0600) Subject: Convert py-tui.c to the "python safety" approach X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c7c7858871f77bb265bbce492d39716f39e0d462;p=thirdparty%2Fbinutils-gdb.git Convert py-tui.c to the "python safety" approach This patch mostly converts py-tui.c to use the new Python safety code. In particular: * All methods of gdb.TuiWindow are now implemented as straightforward methods of gdbpy_tui_window. * gdbpy_register_tui_window is converted and simply returns 'void'. I converted this particular file because it was relatively straightforward, while also demonstrating most of the features of the new approach. For example, explicit result checks aren't needed, try/catch can be removed, and the methods are now written in a natural style. Note that more conversion remains to be done here: * gdbpy_tui_enabled hasn't been converted and still does explicit checks. * There's one explicit check in gdbpy_tui_window::set_title. Fully implementing the safety approach means that some low-level things should eventually be converted to throw; but some work has to be deferred until a lot of the work is complete. Acked-By: Tom de Vries --- diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c index ca0fb89ea49..5a36814debb 100644 --- a/gdb/python/py-tui.c +++ b/gdb/python/py-tui.c @@ -44,13 +44,40 @@ class tui_py_window; /* A PyObject representing a TUI window. */ -struct gdbpy_tui_window: public PyObject +struct gdbpy_tui_window : public PyObject { /* The TUI window, or nullptr if the window has been deleted. */ tui_py_window *window; /* Return true if this object is valid. */ bool is_valid () const; + + /* Require that this object be valid. Throws exception if not. */ + void require_valid () const + { + if (!is_valid ()) + gdbpy_err_format (PyExc_RuntimeError, _("TUI window is invalid.")); + } + + /* Erase the TUI window. */ + void erase (); + + /* Python function that writes some text to a TUI window. */ + void write (gdbpy_borrowed_ref<> args, gdbpy_opt_borrowed_ref<> kw); + + /* Return the width of the TUI window. */ + int width () const; + + /* Return the height of the TUI window. */ + int height () const; + + /* Return the title of the TUI window. */ + const std::string &title () const; + + /* Set the title of the TUI window. */ + void set_title (gdbpy_opt_borrowed_ref<> new_title); + + static PyTypeObject *corresponding_object_type; }; static_assert (gdb::is_python_allocatable_v); @@ -408,173 +435,112 @@ gdbpy_tui_window_maker::operator() (const char *win_name) /* Implement "gdb.register_window_type". */ -PyObject * -gdbpy_register_tui_window (PyObject *self, PyObject *args, PyObject *kw) +void +gdbpy_register_tui_window (gdbpy_borrowed_ref<> args, + gdbpy_opt_borrowed_ref<> kw) { static const char *keywords[] = { "name", "constructor", nullptr }; - const char *name; PyObject *cons_obj; + gdbpy_arg_parse_tuple_and_keywords (args, kw, "sO", keywords, + &name, &cons_obj); - if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "sO", keywords, - &name, &cons_obj)) - return nullptr; - - try - { - gdbpy_tui_window_maker constr (gdbpy_ref<>::new_reference (cons_obj)); - tui_register_window (name, constr); - } - catch (const gdb_exception &except) - { - return gdbpy_handle_gdb_exception (nullptr, except); - } - - return py_none ().release (); + gdbpy_tui_window_maker constr (gdbpy_ref<>::new_reference (cons_obj)); + tui_register_window (name, constr); } -/* Require that "Window" be a valid window. */ - -#define REQUIRE_WINDOW(Window) \ - do { \ - if (!(Window)->is_valid ()) \ - return PyErr_Format (PyExc_RuntimeError, \ - _("TUI window is invalid.")); \ - } while (0) - -/* Require that "Window" be a valid window. */ - -#define REQUIRE_WINDOW_FOR_SETTER(Window) \ - do { \ - if (!(Window)->is_valid ()) \ - { \ - PyErr_Format (PyExc_RuntimeError, \ - _("TUI window is invalid.")); \ - return -1; \ - } \ - } while (0) - -/* Python function which checks the validity of a TUI window - object. */ -static PyObject * -gdbpy_tui_is_valid (PyObject *self, PyObject *args) -{ - gdbpy_tui_window *win = (gdbpy_tui_window *) self; - - if (win->is_valid ()) - return py_true ().release (); - return py_false ().release (); -} - /* Python function that erases the TUI window. */ -static PyObject * -gdbpy_tui_erase (PyObject *self, PyObject *args) +void +gdbpy_tui_window::erase () { - gdbpy_tui_window *win = (gdbpy_tui_window *) self; - - REQUIRE_WINDOW (win); - - win->window->erase (); - - return py_none ().release (); + require_valid (); + window->erase (); } /* Python function that writes some text to a TUI window. */ -static PyObject * -gdbpy_tui_write (PyObject *self, PyObject *args, PyObject *kw) +void +gdbpy_tui_window::write (gdbpy_borrowed_ref<> args, + gdbpy_opt_borrowed_ref<> kw) { + require_valid (); + static const char *keywords[] = { "string", "full_window", nullptr }; - gdbpy_tui_window *win = (gdbpy_tui_window *) self; const char *text; int full_window = 0; - if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, - &text, &full_window)) - return nullptr; - - REQUIRE_WINDOW (win); + gdbpy_arg_parse_tuple_and_keywords (args, kw, "s|i", keywords, + &text, &full_window); - win->window->output (text, full_window); - - return py_none ().release (); + window->output (text, full_window); } -/* Return the width of the TUI window. */ -static PyObject * -gdbpy_tui_width (PyObject *self, void *closure) +int +gdbpy_tui_window::width () const { - gdbpy_tui_window *win = (gdbpy_tui_window *) self; - REQUIRE_WINDOW (win); - gdbpy_ref<> result - = gdb_py_object_from_longest (win->window->viewport_width ()); - return result.release (); + require_valid (); + return window->viewport_width (); } -/* Return the height of the TUI window. */ -static PyObject * -gdbpy_tui_height (PyObject *self, void *closure) +int +gdbpy_tui_window::height () const { - gdbpy_tui_window *win = (gdbpy_tui_window *) self; - REQUIRE_WINDOW (win); - gdbpy_ref<> result - = gdb_py_object_from_longest (win->window->viewport_height ()); - return result.release (); + require_valid (); + return window->viewport_height (); } -/* Return the title of the TUI window. */ -static PyObject * -gdbpy_tui_title (PyObject *self, void *closure) +const std::string & +gdbpy_tui_window::title () const { - gdbpy_tui_window *win = (gdbpy_tui_window *) self; - REQUIRE_WINDOW (win); - return host_string_to_python_string (win->window->title ().c_str ()).release (); + require_valid (); + return window->title (); } /* Set the title of the TUI window. */ -static int -gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure) +void +gdbpy_tui_window::set_title (gdbpy_opt_borrowed_ref<> new_title) { - gdbpy_tui_window *win = (gdbpy_tui_window *) self; + require_valid (); - REQUIRE_WINDOW_FOR_SETTER (win); - - if (newvalue == nullptr) - { - PyErr_Format (PyExc_TypeError, _("Cannot delete \"title\" attribute.")); - return -1; - } + if (new_title == nullptr) + gdbpy_err_format (PyExc_TypeError, + _("Cannot delete \"title\" attribute.")); + /* FIXME: Python safety. Ideally python_string_to_host_string would + throw on error, but this can't be done until more of the code has + been converted. */ gdb::unique_xmalloc_ptr value - = python_string_to_host_string (newvalue); + = python_string_to_host_string (new_title); if (value == nullptr) - return -1; + throw gdb_python_exception (); - win->window->set_title (value.get ()); - return 0; + window->set_title (value.get ()); } static gdb_PyGetSetDef tui_object_getset[] = { - { "width", gdbpy_tui_width, NULL, "Width of the window.", NULL }, - { "height", gdbpy_tui_height, NULL, "Height of the window.", NULL }, - { "title", gdbpy_tui_title, gdbpy_tui_set_title, "Title of the window.", - NULL }, - { NULL } /* Sentinel */ + { "width", wrap_getter, nullptr, + "Width of the window.", nullptr }, + { "height", wrap_getter, nullptr, + "Height of the window.", nullptr }, + { "title", wrap_getter, + wrap_setter, + "Title of the window.", nullptr }, + { nullptr } /* Sentinel */ }; static PyMethodDef tui_object_methods[] = { - { "is_valid", gdbpy_tui_is_valid, METH_NOARGS, + noargs_method ("is_valid", "is_valid () -> Boolean\n\ -Return true if this TUI window is valid, false if not." }, - { "erase", gdbpy_tui_erase, METH_NOARGS, - "Erase the TUI window." }, - { "write", (PyCFunction) gdbpy_tui_write, METH_VARARGS | METH_KEYWORDS, - "Append a string to the TUI window." }, - { NULL } /* Sentinel. */ +Return true if this TUI window is valid, false if not."), + noargs_method + ("erase", "Erase the TUI window."), + varargs_method ("write", + "Append a string to the TUI window."), + { nullptr } /* Sentinel. */ }; PyTypeObject gdbpy_tui_window_object_type = @@ -618,6 +584,9 @@ PyTypeObject gdbpy_tui_window_object_type = 0, /* tp_alloc */ }; +PyTypeObject *gdbpy_tui_window::corresponding_object_type + = &gdbpy_tui_window_object_type; + /* Called when TUI is enabled or disabled. */ static void diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 0793c7fa2af..1165f165e29 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -498,8 +498,9 @@ gdb::unique_xmalloc_ptr gdbpy_parse_command_name (const char *name, struct cmd_list_element ***base_list, struct cmd_list_element **start_list, struct cmd_list_element **prefix_cmd = nullptr); -PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args, - PyObject *kw); + +void gdbpy_register_tui_window (gdbpy_borrowed_ref<> args, + gdbpy_opt_borrowed_ref<> kw); gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal); gdbpy_ref<> symtab_to_symtab_object (struct symtab *symtab); diff --git a/gdb/python/python.c b/gdb/python/python.c index 214abd03905..7b5de98b903 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -3274,10 +3274,9 @@ or None if not set." }, Set the value of the convenience variable $NAME." }, #ifdef TUI - { "register_window_type", (PyCFunction) gdbpy_register_tui_window, - METH_VARARGS | METH_KEYWORDS, + varargs_function ("register_window_type", "register_window_type (NAME, CONSTRUCTOR) -> None\n\ -Register a TUI window constructor." }, +Register a TUI window constructor."), #endif /* TUI */ { "architecture_names", gdbpy_all_architecture_names, METH_NOARGS,