From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x12d.google.com (mail-il1-x12d.google.com [IPv6:2607:f8b0:4864:20::12d]) by sourceware.org (Postfix) with ESMTPS id 88AD03858D1E for ; Wed, 17 May 2023 15:32:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 88AD03858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-il1-x12d.google.com with SMTP id e9e14a558f8ab-3382b8b3c33so6809825ab.2 for ; Wed, 17 May 2023 08:32:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1684337543; x=1686929543; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=LsZkQ42Bn4AnqWLsj+Ae9kfA0oX4OBBlOz+rSYImu/E=; b=cyApW9xekhTDAMDTQU04H90KAycPlqwvVE6BwYSgionDdFzyDn0Lov7E1Nyldl2wkA vuZT8VT39CxXGpZOlsp/GbPyteXurLLSPmpkufA4q0F5D45bOEfc7MqxeLhT7kVtsbUB zlJHVxhUPOGzXE0ykyA/lW9i+NbeHcutJf9yYu2UXk/LNhgNtaXBTi3l1OC1nUyEq7Tp b0kOf3QVWTtzbW2bYhSlxpu7NB0PShuqWX+NKaxD9m0mkqYjzG2Pii7urXhOzxEB1rk5 +BpZvhYfUYFC5EMMWxvzwXDlAP7lXzNdVDt4MJhOr3xz2XFjucTpTCR1djIgundxTHgt zp+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684337543; x=1686929543; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=LsZkQ42Bn4AnqWLsj+Ae9kfA0oX4OBBlOz+rSYImu/E=; b=NHl81/Ia/wCEgfzKcZo/uFI+/hM22KtcUVJyizEwd+/aw1PQh+cK8Ao3EHc+RynkY2 uGFABKVhkQXHcOuyfThm4n2WmPetltpQGRTuFFzyaNhrJiMbB/yr2GybA5YXTKPrqWWp LkHL4meEPQnJO2+ZbrZW7FYVukIGnr3prO5B9MzBriyWW6AJBncAqshzCj6OhYnF7Iwo 2/7cSmd/j3+i0yto2n6QamYVaRh+6bgUvMJzgspO5mOMqZveTAJMy8dXehZ13w/wSBKJ CKQCVszm4H3kuflq46XS8k/QVmqEtcByWtN6uN2y3zOZfJBQQbSS5I+ydd3379E9Y4Vi Qd1w== X-Gm-Message-State: AC+VfDxiho8d4XqxFZX31bybwVb0li1rh2mAs3uMzeDW/YwoxHY2xRJC iUU1GNIbczmZxrIqnWEj5GeHeqRzXvxTq8yaAkvAFw== X-Google-Smtp-Source: ACHHUZ55eH/X70jxJNJrjZOUSaQIoOTSQNfsexaGRapLf43nMa/NxIO59XFq7/2TyHAWHyOOPfQ9vQ== X-Received: by 2002:a05:6e02:683:b0:330:5071:748d with SMTP id o3-20020a056e02068300b003305071748dmr2396023ils.23.1684337543502; Wed, 17 May 2023 08:32:23 -0700 (PDT) Received: from murgatroyd (75-166-157-55.hlrn.qwest.net. [75.166.157.55]) by smtp.gmail.com with ESMTPSA id c6-20020a029606000000b004050767f779sm8681787jai.164.2023.05.17.08.32.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 May 2023 08:32:22 -0700 (PDT) From: Tom Tromey To: Andrew Burgess Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Add attributes and methods to gdb.Inferior References: <20230509-dap-args-v1-0-16b5f0aa5cd6@adacore.com> <20230509-dap-args-v1-1-16b5f0aa5cd6@adacore.com> <87edno7aoa.fsf@redhat.com> X-Attribution: Tom Date: Wed, 17 May 2023 09:32:22 -0600 In-Reply-To: <87edno7aoa.fsf@redhat.com> (Andrew Burgess's message of "Wed, 10 May 2023 11:17:25 +0100") Message-ID: <87ednf0y9l.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: >>>>> "Andrew" == Andrew Burgess writes: >> + const char *name; >> + if (!PyArg_ParseTuple (args, "s", &name)) Andrew> I'd really prefer gdb_PyArg_ParseTupleAndKeywords be used here too. I Andrew> know there's only a single argument, but I think it would be nice if our Andrew> API was consistent in always accepting keywords. Ok, I did this. gdb doesn't generally do this for single-argument functions, but it's also relatively harmless to do so ("relatively" since it does mean the name is ABI). >> + if (value == nullptr) >> + { >> + PyErr_SetString (PyExc_TypeError, _("Cannot delete 'arguments' attribute.")); Andrew> Line length? Fixed. >> + return -1; >> + } >> + >> + if (PyUnicode_Check (value)) Andrew> I prefer gdbpy_is_string, which is used more. Though Andrew> PyUnicode_Check is present in a few places. I made this change, but it seems to me that gdbpy_is_string was probably there for Python 2/3 compatibility. Now that it's just a simple wrapper for PyUnicode_Check, it seems like it would be more "Pythonically idiomatic" to just remove the wrapper. >> + /* This is unfortunate but the implementation of main_name can >> + reach into memory, among other things. */ >> + scoped_restore_current_inferior restore_inferior; >> + set_current_inferior (inf->inferior); >> + >> + scoped_restore_current_program_space restore_current_progspace; >> + set_current_program_space (inf->inferior->pspace); Andrew> I guess switch_to_inferior_no_thread would be overkill here, if all Andrew> we're doing is accessing inferior memory? Though you do tease us with Andrew> "among other things", so hopefully nothing is using the incorrect Andrew> thread.... I removed the "among other things" bit. I think the thread shouldn't really matter. The only offender here is Ada and there's a bug open about how it would be better to get this info from the executable instead. >> + >> + name = main_name (); >> + } >> + catch (const gdb_exception &except) >> + { >> + /* We can just ignore this. */ >> + name = nullptr; Andrew> Is this assignment necessary, given name is initialised to nullptr? Apparently not, I removed it in v2. Tom