From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +MOLDRRF0GK1zhIAWB0awg (envelope-from ) for ; Thu, 14 Jul 2022 12:32:20 -0400 Received: by simark.ca (Postfix, from userid 112) id 286AF1E5EA; Thu, 14 Jul 2022 12:32:20 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=Pi2G0upl; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B2B651E21F for ; Thu, 14 Jul 2022 12:32:19 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 490B93858419 for ; Thu, 14 Jul 2022 16:32:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 490B93858419 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1657816339; bh=E18MxFjUmTN73U9FbB/IZY11qeSP1TXhtgizqez+beg=; h=To:Subject:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Pi2G0upl5LuGoIQ8tJbe9/cl1Ss7Lt1UqxwMv8iv9hdfDZYrKoAEkl4D3yGDx/li1 /49L2OI9ndKLZq3YhAbdmDmTTcC1XY9XfT/B1yZMFWTAmgKdxicrpkGPLTrkLBEEwt SkBqQf2me+RCeE3ZnkcV7brznPzEQuTZmjxWOfMM= Received: from mail-io1-xd31.google.com (mail-io1-xd31.google.com [IPv6:2607:f8b0:4864:20::d31]) by sourceware.org (Postfix) with ESMTPS id A055E3858D1E for ; Thu, 14 Jul 2022 16:31:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A055E3858D1E Received: by mail-io1-xd31.google.com with SMTP id p81so1855148iod.2 for ; Thu, 14 Jul 2022 09:31:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=E18MxFjUmTN73U9FbB/IZY11qeSP1TXhtgizqez+beg=; b=v6vho9K8z7XT5CpX/XptMaPbkKXFplCSjYElKFazQF8fOAO2tsw25JqntRp7gvLXkU jocXixo1pIkRsHB/A72HEqw+lQZEtpDYbCFkmcxKlDkcUg3++F4FDA0o7CGPrsd8nPuR 6zGID+wRR92kQFbJVI8Kn3oT82dXiIcEHMYlEb6R5mEA1ZzuWrLzFPAw6nfbAkYnpCGm D9prVo7EEHbpPyKzNNM8SAzlmBW6b/OXBujBtDt9LxuEwOQ+AhuRhbYvue418LKVw5EE Ra+VuqMV7zn0NcCRs/ywZTD8QAVCPhAdQfBiLjm9N72z3tNz+moPOwbRh1F74mTTrH6N EdlA== X-Gm-Message-State: AJIora/bDVZ1y0V8LdPk8ofNqtfi93paVLAhMDaA1ZJXq0HGKoQMYqie GOXDHw5S7lgqOKMZhmeehaXTew== X-Google-Smtp-Source: AGRyM1vqbxVabcB4BRi3au/dWOM5a9bt2R1JzTIWdfCMaxtrf9XwgMFUmcKJ3p1OUHowyzjtFMwRJQ== X-Received: by 2002:a05:6638:25c4:b0:33e:a176:c227 with SMTP id u4-20020a05663825c400b0033ea176c227mr5057384jat.61.1657816317884; Thu, 14 Jul 2022 09:31:57 -0700 (PDT) Received: from murgatroyd (71-211-185-228.hlrn.qwest.net. [71.211.185.228]) by smtp.gmail.com with ESMTPSA id w26-20020a05663800da00b003314d7b59b0sm907848jao.88.2022.07.14.09.31.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jul 2022 09:31:57 -0700 (PDT) To: Simon Farre via Gdb-patches Subject: Re: [PATCH v7] gdb/python: Add BreakpointLocation type References: <20220607115748.484438-1-simon.farre.cx@gmail.com> X-Attribution: Tom Date: Thu, 14 Jul 2022 10:31:56 -0600 In-Reply-To: <20220607115748.484438-1-simon.farre.cx@gmail.com> (Simon Farre via Gdb-patches's message of "Tue, 7 Jun 2022 13:57:48 +0200") Message-ID: <87r12nslj7.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom Tromey via Gdb-patches Reply-To: Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" >>>>> "Simon" == Simon Farre via Gdb-patches writes: Simon> +static int find_loc_num_by_location (bp_location* loc); I think it would be better to rearrange this so that the forward declaration isn't needed. Simon> +@defvar Breakpoint.locations Simon> +Get the most current list of breakpoint locations that are inserted for this Simon> +breakpoint, with elements of type @code{gdb.BreakpointLocation} Simon> +(described below). This functionality matches that of the Simon> +@code{info breakpoint} command (@pxref{Set Breaks}), in that it only retrieves Simon> +the most current list of locations, thus the list itself when returned is Simon> +not updated behind the scenes. This attribute is not writable. Simon> +@end defvar I used this patch for some work I'm doing and I found this description a bit hard to follow. Maybe the "This functionality..." sentence could be replaced with one that just says "This does not cause gdb to update the underlying list of locations"? Simon> +@defvar BreakpointLocation.source Simon> +This attribute returns the source file path and line number where this location Simon> +was set. The type of the attribute is a tuple of @var{string} and Simon> +@var{long}. If the breakpoint location doesn't have a source location, I think nowadays there is only 'int' in Python. This occurs a bit later in the text as well. Simon> +/* Python function to get the breakpoint locations of an owner breakpoint. */ Simon> + Simon> +static PyObject* Space before "*". Simon> +static PyObject * Simon> +bplocpy_get_source_location (PyObject *py_self, void *closure) Simon> +{ [...] Simon> + auto line = gdb_py_object_from_ulongest (self->bp_loc->line_number); I think a null check is needed here. Simon> +static PyObject * Simon> +bplocpy_get_thread_groups (PyObject *py_self, void *closure) [...] Simon> + if (PyList_Append (list.get (), Simon> + gdb_py_object_from_ulongest(inf->num).release()) != 0) Simon> + return nullptr; The formatting looks weird here, but also gdb_py_object_from_ulongest has to be called separately and the result checked for nullptr. Note there's also a missing space before the "(". Simon> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h Simon> index d947b96033b..7c4648e6042 100644 Simon> --- a/gdb/python/python-internal.h Simon> +++ b/gdb/python/python-internal.h Simon> @@ -322,12 +322,10 @@ struct gdbpy_breakpoint_object Simon> } \ Simon> } while (0) Simon> - Simon> /* Variables used to pass information between the Breakpoint Simon> constructor and the breakpoint-created hook function. */ Simon> extern gdbpy_breakpoint_object *bppy_pending_object; Simon> - Simon> struct thread_object Simon> { Simon> PyObject_HEAD This seems like a spurious change. Simon> + Copyright 2022-2022 Free Software Foundation, Inc. Don't need a range here. Simon> --- /dev/null Simon> +++ b/gdb/testsuite/gdb.python/py-bp-locations.exp Simon> @@ -0,0 +1,64 @@ Simon> +# Copyright (C) 2022-2022 Free Software Foundation, Inc. Here either. thanks, Tom