From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 0RgFF9wIC2rBdwUAWB0awg (envelope-from ) for ; Mon, 18 May 2026 08:41:00 -0400 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EevD/R7V; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 58EA61E024; Mon, 18 May 2026 08:41:00 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_INVALID,DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3D2A61E024 for ; Mon, 18 May 2026 08:40:59 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9CF7F4BB1C3A for ; Mon, 18 May 2026 12:40:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9CF7F4BB1C3A Authentication-Results: sourceware.org; dkim=fail reason="signature verification failed" (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EevD/R7V Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id BDE8C4BB1C09 for ; Mon, 18 May 2026 12:39:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BDE8C4BB1C09 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BDE8C4BB1C09 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779107978; cv=none; b=EqaufcabHTQNTqdo4GlF18ivYjL87ON/AldqB8ZHbUdFFyITmQGtFM0nF4UimvUQczW1v8m9wRNx1R4CkPO4wHzB57VxBagInbiAn1O6Q9CbdXWm6QP17uv1rPyjVXOMr/MBSkcTADqd1p+wnzXrRgzkgv5vQMDHD2l4WonR4uo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779107978; c=relaxed/simple; bh=WAZtEAP9Z4F3aI/BEbJqQtl4sZZEfRAVesy9NVzxIDg=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=JTnAWO5fXelwNqstUlM5+TGA55OZdKyBvKugAMTQos0TRPmuC9vz6dJL5kqF3UDuhCkU8BQqDWRMB1biucX3cITnd8kt8ib89m2uvXc4HCoLr/e7zQLQUgnEcneU+meG9kdJRAzF61OgVxGzIIV4BqPNv7vGUIfs8xWPetJ7WI4= ARC-Authentication-Results: i=1; sourceware.org; dkim=fail (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EevD/R7V reason="signature verification failed" DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BDE8C4BB1C09 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1779107978; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tQufcJ0BuqeE/PA8onS7PfqlTjgtaCKgrzMVHhMx60c=; b=EevD/R7VK6E2KNra41/CGLLNNYrNzWXK+F4TBkjw6DuBmMTpU4Mm3KCPkl+gO0FR0BAv8T MgONLm62jj/vTfAHit+mnxq0tXp4E61kWVf74IWJxEnUqwZn/aSa2j5J9Q40KVzy7K9rcd U2TXRibOkbo5XrTARgbDAWDjCk/j918= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-632-bkiY_BzWNr6hb9u65dOGYQ-1; Mon, 18 May 2026 08:39:37 -0400 X-MC-Unique: bkiY_BzWNr6hb9u65dOGYQ-1 X-Mimecast-MFC-AGG-ID: bkiY_BzWNr6hb9u65dOGYQ_1779107976 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-44d83e45febso2310113f8f.0 for ; Mon, 18 May 2026 05:39:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779107976; x=1779712776; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=wIlgwFp+2sLpFvhwTfGY9ahYePlp2FQj1r3z6/uhOkA=; b=L5qWyig/bPpQDwDC1zFgK3o1JSWDQa/Mh5StAc6iPb+fSAei0ga5DjdkQWBVk8+EQ3 f5g3Vm7wopdP72nuuyyxuCqqFIBNrrNh4G0Dw9pW7yJR8JuHpl/zwwdTjbaxqYdOqQ8v bN/GmzqeR8c+LT1HrcjYDw/5KJnMYL9EJhmXMoBMs8x90blv3fz0+5Ei14o7/LoKbjYU l4DuL5nLH/Tq2F+8AJtVeprDDg8Wh2TgL0vGK3AAeuTv8gq2/7mUbxMQmCITm0QAYSbH XkJUEHEBl91wYBSob+7Nfj59UBVSnYvLuiq6sJWx1gN1k7MlxsD8+jQA+XFfilerlx+5 BuIQ== X-Forwarded-Encrypted: i=1; AFNElJ9wjwsdFBb9Lakf1F9I2K25Lro9G13PFLrK3hb036WEKqqc9Y2jpbzHDozbgxOV51h2fgJBYPtpqxqvzA==@sourceware.org X-Gm-Message-State: AOJu0YxYc65HIwihPHNVk8jJxYujjfCoxC8x4/dXNuqNbek/9QbCQGay eHZOEdxVAOvDIt99CEXsCpUueRV6JMvDTbMboJUyiiy0n8FXFZID4prR9LpoG3By3TPvlH4Dyr2 lUlyww87H5EulblcrNR4arAblakFyYZJFgVGGYfY+mx6V+kc/KcylDuQyaxF95cc= X-Gm-Gg: Acq92OFNT5rIBPNsmvtNgqwMvr5uhHAuWAHEzzLW8RJLN3KPTVcorknUTRuDb2FF7J8 1l5iJXDrCuT604s02zWVs3JHRvj1nkQYwrVIV+8gsNSs/poNOt75zTBgChJsPFIGsFT9HyyVSd3 kqxONbLZezb0EVTrw/hxP4n6sdQIxaR0prMjHM9UQsAam0oEWnn7UoxLKalR3XXtnn2ybRuyjHH r+leUOQmdWGQZho/+kkCIRon8IidEbx7t1uGnir8gjZxHpvATFNv27EEQ/pvQTw/264HJ3QFTVo 3il7ZxArF8UrETIzI7ndxW++YESZBFictrm+ujGMmvNVAPqCZr92HEq1dD2I1oBlKuXq0wMQKdO KVHX4guLVeJ/OWBOrasI4iWRvlIc= X-Received: by 2002:a05:600c:83c7:b0:48f:e230:29f5 with SMTP id 5b1f17b1804b1-48fe53a8eccmr224564595e9.16.1779107975561; Mon, 18 May 2026 05:39:35 -0700 (PDT) X-Received: by 2002:a05:600c:83c7:b0:48f:e230:29f5 with SMTP id 5b1f17b1804b1-48fe53a8eccmr224563835e9.16.1779107975012; Mon, 18 May 2026 05:39:35 -0700 (PDT) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fe53767ecsm230818575e9.10.2026.05.18.05.39.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2026 05:39:34 -0700 (PDT) From: Andrew Burgess To: Tom Tromey , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH v2 4/4] Convert py-tui.c to the "python safety" approach In-Reply-To: <20260515-python-safety-initial-v2-4-6129cadf258a@tromey.com> References: <20260515-python-safety-initial-v2-0-6129cadf258a@tromey.com> <20260515-python-safety-initial-v2-4-6129cadf258a@tromey.com> Date: Mon, 18 May 2026 13:39:33 +0100 Message-ID: <871pf8dh8a.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: fDTmARuf3h5EKPGNqO2WwYqvWrWmERvJTNT9zafLdlA_1779107976 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom Tromey writes: > 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. > > * gdb.register_tui_window is converted and simply returns 'void'. typo: gdb.register_tui_window should be gdbpy_register_tui_window I think. > > 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. I like the new approach, and think it looks like a great improvement over the existing code. > --- > gdb/python/py-tui.c | 208 ++++++++++++++++++------------------------- > gdb/python/python-internal.h | 5 +- > gdb/python/python.c | 5 +- > 3 files changed, 92 insertions(+), 126 deletions(-) > > diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c > index 111e0f4c9e3..0755d7b92e3 100644 > --- a/gdb/python/py-tui.c > +++ b/gdb/python/py-tui.c > @@ -50,7 +50,34 @@ struct gdbpy_tui_window: public PyObject > tui_py_window *window; > > /* Return true if this object is valid. */ > - bool is_valid () const; > + bool is_valid (); Dropping 'const' here is unfortunate, and I think is a consequence of either noargs_method or maybe wrapped_method requiring non-const. Can we add a 'const' aware overload of noargs_method, or wrapped_method, or whatever, and allow is_valid to retain the const qualifier? I think it would be a shame if a consequence of this work is that we end up dropping the const qualifiers in many places. If this works, then there might be other places in the wrapper functions (patch #3) where we could/should add const overloads. > + > + /* Require that this object be valid. Throws exception if not. */ > + void require_valid () > + { > + 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 (); > + > + /* Return the height of the TUI window. */ > + int height (); > + > + /* Return the title of the TUI window. */ > + const std::string &title (); > + > + /* Set the title of the TUI window. */ > + void set_title (gdbpy_opt_borrowed_ref new_title); > + > + static PyTypeObject *corresponding_object_type; > }; > > extern PyTypeObject gdbpy_tui_window_object_type; > @@ -150,7 +177,7 @@ class tui_py_window : public tui_win_info > /* See gdbpy_tui_window declaration above. */ > > bool > -gdbpy_tui_window::is_valid () const > +gdbpy_tui_window::is_valid () > { > return window != nullptr && tui_active; > } > @@ -406,173 +433,109 @@ 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 () > { > - 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 () > { > - 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 () > { > - 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: safety */ Please can you expand this comment. I know this is something you're actively working on, and the plan is that this comment probably shouldn't exist in the code for too long. But it might, so we should assume that it will, and write this accordingly. For me, and 'FIXME' command should explain (1) what needs fixing, and (2) why it cannot currently be fixed. Armed with these two pieces of information I can, in the future, make an informed decision about whether the issue has been, or can now, be fixed. I'll be honest, even with the commit message hint, I don't really understand what it is that needs fixing here, I guess it's that you'd like python_string_to_host_string to throw rather than return NULL? But the issue is that this function is used in non-safety code, so cannot (currently) be changed? So we'd eventually have: gdb::unique_xmalloc_ptr value = python_string_to_host_string (new_title); gdb_assert (value != nullptr); ... etc ... Thanks, Andrew