From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25056 invoked by alias); 22 Sep 2017 18:46:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 25040 invoked by uid 89); 22 Sep 2017 18:46:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 22 Sep 2017 18:46:01 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 568A5C0828D2; Fri, 22 Sep 2017 18:46:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 568A5C0828D2 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F0832600C8; Fri, 22 Sep 2017 18:45:59 +0000 (UTC) From: Sergio Durigan Junior To: Eli Zaretskii Cc: gdb-patches@sourceware.org, palves@redhat.com Subject: Re: [PATCH v3 5/5] Extend "set cwd" to work on gdbserver References: <20170912042325.14927-1-sergiodj@redhat.com> <20170921225926.23132-1-sergiodj@redhat.com> <20170921225926.23132-6-sergiodj@redhat.com> <83o9q3cftp.fsf@gnu.org> Date: Fri, 22 Sep 2017 18:46:00 -0000 In-Reply-To: <83o9q3cftp.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 22 Sep 2017 11:12:18 +0300") Message-ID: <87bmm21sig.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00692.txt.bz2 On Friday, September 22 2017, Eli Zaretskii wrote: >> From: Sergio Durigan Junior >> Cc: Pedro Alves , Sergio Durigan Junior >> Date: Thu, 21 Sep 2017 18:59:26 -0400 >> >> This is the "natural" extension necessary for the "set cwd" command >> (and the whole "set the inferior's cwd" logic) to work on gdbserver. >> >> The idea here is to have a new remote packet, QSetWorkingDir (name >> adopted from LLDB's extension to the RSP, as can be seen at >> ), >> which sends an hex-encoded string representing the working directory >> that gdbserver is supposed to cd into before executing the inferior. >> The good thing is that since this feature is already implemented on >> nat/fork-inferior.c, all gdbserver has to do is to basically implement >> "set_inferior_cwd" and call it whenever such packet arrives. > > This once again raises the issue of whether to send expanded or > unexpanded directory down the wire, and if unexpanded, then what is > the meaning of the default "~" in the inferior. FWIW, I decided (based on another message by Pedro) to modify the behaviour of "set cwd" without arguments. Before it was setting the inferior's cwd as "~", but now it clears out whatever cwd that has been specified by the user. But of course, the user can still do "set cwd ~". We do not expand paths on the host, as I explained in another message, so gdbserver will see "~" coming down the wire. Then, when the inferior is to be started, gdbserver will perform the path expansion based on what "gdb_tilde_expand" (i.e., "glob") does. >> + if (inferior_cwd != NULL) >> + { >> + size_t cwdlen = strlen (inferior_cwd); >> + >> + wcwd = alloca ((cwdlen + 1) * sizeof (wchar_t)); >> + mbstowcs (wcwd, inferior_cwd, cwdlen + 1); >> + } > > no error checking of the mbstowcs conversion? Sorry, I am not a Windows programmer. Other places in the code also don't check for errors. I'd be happy to improve this code, but I refuse to touch a Windows machine so I'm doing this all this without any testing. But please, feel absolutely free to point out how this code should look like. >> ret = CreateProcessW (wprogram, /* image name */ >> wargs, /* command line */ >> NULL, /* security, not supported */ >> @@ -586,7 +595,7 @@ create_process (const char *program, char *args, >> FALSE, /* inherit handles, not supported */ >> flags, /* start flags */ >> NULL, /* environment, not supported */ >> - NULL, /* current directory, not supported */ >> + wcwd, /* current directory */ >> NULL, /* start info, not supported */ >> pi); /* proc info */ >> #else >> @@ -599,7 +608,7 @@ create_process (const char *program, char *args, >> TRUE, /* inherit handles */ >> flags, /* start flags */ >> NULL, /* environment */ >> - NULL, /* current directory */ >> + inferior_cwd, /* current directory */ >> &si, /* start info */ >> pi); /* proc info */ > > Once again, this feeds CreateProcess with an unexpanded directory, > which AFAIU will not work if the directory includes "~". You're right; I've changed that now. >> +static void >> +extended_remote_set_inferior_cwd (struct remote_state *rs) >> +{ >> + if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE) >> + { >> + const char *inferior_cwd = get_inferior_cwd (); >> + >> + if (inferior_cwd != NULL) >> + { >> + std::string hexpath = bin2hex ((const gdb_byte *) inferior_cwd, >> + strlen (inferior_cwd)); >> + > > Shouldn't this do some encoding conversion, from the GDB charset to > the target charset, before encoding in hex? I don't know. There's nothing related to charset on gdb/gdbserver/, but then again I don't know if we've ever encountered a case that demanded conversions. I can investigate this a bit more. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/