From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 884A73857C5E for ; Mon, 15 Nov 2021 13:16:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 884A73857C5E Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 1AFDGO4c032417 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 15 Nov 2021 08:16:28 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 1AFDGO4c032417 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id BDA881E940; Mon, 15 Nov 2021 08:16:23 -0500 (EST) Message-ID: Date: Mon, 15 Nov 2021 08:16:23 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCHv5 3/4] gdb/python: add gdb.RemoteTargetConnection.send_packet Content-Language: en-US To: Andrew Burgess Cc: Andrew Burgess , gdb-patches@sourceware.org References: <20211115092509.GI2352@redhat.com> From: Simon Marchi In-Reply-To: <20211115092509.GI2352@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 15 Nov 2021 13:16:24 +0000 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Mon, 15 Nov 2021 13:16:37 -0000 On 2021-11-15 04:25, Andrew Burgess wrote: > * Simon Marchi via Gdb-patches [2021-11-14 21:08:40 -0500]: > >> Just some minor comments (one about a possible bug), but otherwise this >> looks fine to me. >> >>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >>> index 2234bbe1891..0fd31ca6cb3 100644 >>> --- a/gdb/doc/python.texi >>> +++ b/gdb/doc/python.texi >>> @@ -5989,9 +5989,9 @@ >>> Examples of different connection types are @samp{native} and >>> @samp{remote}. @xref{Inferiors Connections and Programs}. >>> >>> -@value{GDBN} uses the @code{gdb.TargetConnection} object type to >>> -represent a connection in Python code. To get a list of all >>> -connections use @code{gdb.connections} >>> +Connections in @value{GDBN} are represented as instances of >>> +@code{gdb.TargetConnection}, or as one of its sub-classes. To get a >>> +list of all connections use @code{gdb.connections} >>> (@pxref{gdbpy_connections,,gdb.connections}). >>> >>> To get the connection for a single @code{gdb.Inferior} read its >>> @@ -6044,6 +6044,40 @@ >>> to the remote target. >>> @end defvar >>> >>> +The @code{gdb.RemoteTargetConnection} class is a sub-class of >>> +@code{gdb.TargetConnection}, and is used to represent @samp{remote} >>> +and @samp{extended-remote} connections. In addition to the attributes >>> +and methods available from the @code{gdb.TargetConnection} base class, >>> +a @code{gdb.RemoteTargetConnection} has the following method: >> >> In your opinion, would it be ok (backwards-compatible-wise) to introduce >> a gdb.ExtendedRemoteTargetConnection class in the future, should we ever need >> that? The only case that I think of that would break is if somebody >> does: >> >> type(my_target) is gdb.RemoteTargetConnection >> >> where my_target is an extended-remote connection. That expression would >> return True today, and would return False after adding >> gdb.ExtendedRemoteTargetConnection. But if that could would use >> isinstance, then it would be fine. > > I did worry about this too. But I don't think we should limit > ourselves to just worrying about remote/extended-remote targets, what > if, in the future, we wanted special handling for native targets, or > simulator targets, or any other target type.... > > I did consider creating different object types for each possible > target right up front, but figured that might be overkill. Given > you're also concerned about this, maybe we can avoid the problem by > adding this text to the manual: > > Currently there is only a single sub-class of > @code{gdb.TargetConnection}, @code{gdb.RemoteTargetConnection}, > however, additional sub-classes may be added in future releases of > @value{GDBN}. As a result you should avoid writing code like: > > @smallexample > conn = gdb.selected_inferior().connection > if type(conn) is gdb.RemoteTargetConnection: > print("This is a remote target connection") > @end smallexample > > @noindent > As this may fail when more connection types are added. Instead, you I would say "As this may fail if @code{gdb.RemoteTargetConnection} sub-classes are added in the future". > should write: > > @smallexample > conn = gdb.selected_inferior().connection > if isinstance(conn, gdb.RemoteTargetConnection): > print("This is a remote target connection") > @end smallexample > > We basically reserve the right to add additional connection type > classes in the future, and tell folk not to use 'type(object)' but > always use 'isinstance(object, type)'. If they do break the rules > it's still not a massive task to fix their code, and if they do follow > the rules, they'll be fine. > > What do you think? That sounds good to me. I also thought about creating all types up front, but that would just cover the target connection types that exist today, what if a new one is introduced in the future? Not that likely, but still. Simon