From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 404983858C39 for ; Tue, 9 Jan 2024 16:30:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 404983858C39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gnu.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gnu.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 404983858C39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:470:142:3::10 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704817852; cv=none; b=Pywfc4jMewWdA0bb229Q9yHYFf0WeHqVxNArKAt+EK/Q3KJqVanIV67kkjHrghWD5dQVvnB73sL5g8CViwH58rPThklPg6kMvnRa6q9u3eSCXDrFhLoH7ey14C7mOOS545Cqz22k5OLPg+OlfialAt3zPOD6w8bncuBwZYyVdy8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704817852; c=relaxed/simple; bh=UR4jkipsDZPdCFK3fdkLPZ4sjeUIU7Qmkti4gV6sEc0=; h=DKIM-Signature:Date:Message-Id:From:To:Subject; b=f1tbEQYhCJFr0s2fZGAmEpoBNVhRWLzbJgYEX12eK0M3K8+D+MileNuPu+8GZ4tJ1+u+rZq6EcBwW07oNwuyebaS5qUTvBYSNx3Nem0NcqtMPxSh9+VuOuvU8jT4hQST3gRnOEvvKJP8A6pCpOdJ66Xk0tdNcNYinxdTucVzeuI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rNF05-0007Yu-JP; Tue, 09 Jan 2024 11:30:49 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=eqfPxmAtcioLHK6Hrw5Yqj7Rv+uAljUI1PiNldeHUSA=; b=DRH/fehpRRsp kWf119M3cWCXN4Z79UPy+R6wFdQEuPtrkYeTelvMlZ12rw9wO5N+KkY4wxujJh/GaIEo2gFYxigkI OlqLZPNVkQKW7vJFN8JNNHdS3OnuDRr1iyCznrBgq5vTXFklmDfEV420U+1GiLDWVOPAaOeHhURPz /gT5gqXxLeB1+P+EHb5A9y+tpwG1Ir7aaIeFoas8QcbBU7weheKuJ6GMzg0UWqegtoiDM9iHEy2Tf kEV9Zae3opVDOnC6tQuZ9qJkOFQnXyRTC2/uL4bl8pTi7ECgC+6tGXuFCYN8TgutjY+O+qXcUQ9mz jRWPCE7Ulbc65tYwIFrKQA==; Date: Tue, 09 Jan 2024 18:30:28 +0200 Message-Id: <83frz6pizf.fsf@gnu.org> From: Eli Zaretskii To: Andrew Burgess Cc: gdb-patches@sourceware.org, m.weghorn@posteo.de In-Reply-To: (message from Andrew Burgess on Tue, 9 Jan 2024 14:26:32 +0000) Subject: Re: [PATCH 09/16] gdb/python: change escaping rules when setting arguments References: X-Spam-Status: No, score=-0.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,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: > From: Andrew Burgess > Cc: Andrew Burgess , Michael Weghorn > Date: Tue, 9 Jan 2024 14:26:32 +0000 > > It is possible to set an inferior's arguments through the Python API > by assigning to the gdb.Inferior.arguments attribute. > > This attribute can be assigned a string, in which case the string is > taken verbatim as the inferior's argument string. Or this attribute > can be assigned a sequence, in which case the members of the sequence > are combined (with some escaping applied) to create the inferior's > argument string. > > Currently, the when the arguments from a Python list are escaped, we > use escape_shell_characters. I suspect the reasons for this are > mostly accidental. > > When the gdb.Inferior.arguments attribute was introduced in commit: > > commit 3153113252f3b949a159439a17e88af8ff0dce30 > Date: Mon May 1 13:53:59 2023 -0600 > > Add attributes and methods to gdb.Inferior > > GDB's inferior::set_args method called construct_inferior_arguments, > and construct_inferior_arguments didn't take an escaping function as a > parameter, the only option was escape_shell_characters as that was the > escaping hard-coded into construct_inferior_arguments. The commit > message makes no comments for or against escaping of special shell > characters, and no tests were added that checked this behaviour. All > of this leads me to think that the handling of special shell > characters wasn't really considered (an understandable oversight). > > But I'd like to consider it, and I think the current behaviour is not > ideal. > > Consider this case: > > (gdb) python gdb.selected_inferior().arguments = ['$VAR'] > (gdb) show args > Argument list to give program being debugged when it is started is "\$VAR". > > This means that when the inferior is run it will see literal '$VAR' as > its argument. If instead, the user wants to pass the shell expanded > value of $VAR to the inferior, there's no way to achieve this result > using the list assignment method. > > In this commit I propose that we change this behaviour so that we > instead see this: > > (gdb) python gdb.selected_inferior().arguments = ['$VAR'] > (gdb) show args > Argument list to give program being debugged when it is started is "$VAR". > > Now the '$' character is not escaped. If the inferior is started > under a shell then the user will see the shell expanded value of > '$VAR'. > > Of course, if the user wants to pass a literal '$VAR' (with no > expansion) then they can do: > > (gdb) python gdb.selected_inferior().arguments = ['\$VAR'] > > This actually feels more natural to me, the user writes the argument > as they would present it to a shell. > > So, after this commit, GDB only escapes quote characters and white > space characters. This keeps some level of backward compatibility > with the existing behaviour (for things other than shell special > characters), but also seems natural, from the user's point of view, > the arguments they are providing are already quoted (by Python's > string quotes) so there's no need to quote white space. It's only > when GDB converts the Python sequence into a single string that the > white space actually needs quoting. > > There are tests for the updated functionality, and I've updated the > docs and added a NEWS entry. > --- > gdb/NEWS | 4 +++ > gdb/doc/python.texi | 7 +++-- > gdb/python/py-inferior.c | 2 +- > gdb/testsuite/gdb.python/py-inferior.exp | 36 ++++++++++++++++++++---- > gdbsupport/common-inferior.cc | 14 +++++++++ > gdbsupport/common-inferior.h | 5 ++++ > 6 files changed, 60 insertions(+), 8 deletions(-) The documentation parts are okay, thanks. Reviewed-By: Eli Zaretskii