From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31755 invoked by alias); 3 Oct 2017 15:15:23 -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 31696 invoked by uid 89); 3 Oct 2017 15:15:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_1,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=inherited, rereading 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; Tue, 03 Oct 2017 15:15:20 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0B72E7E392; Tue, 3 Oct 2017 15:15:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0B72E7E392 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2C0C85D729; Tue, 3 Oct 2017 15:15:15 +0000 (UTC) Subject: Re: [PATCH v5 2/3] Implement "set cwd" command on GDB To: Sergio Durigan Junior , GDB Patches References: <20170912042325.14927-1-sergiodj@redhat.com> <20170929225852.21872-1-sergiodj@redhat.com> <20170929225852.21872-3-sergiodj@redhat.com> Cc: Eli Zaretskii From: Pedro Alves Message-ID: Date: Tue, 03 Oct 2017 15:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170929225852.21872-3-sergiodj@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-10/txt/msg00047.txt.bz2 Hi Sergio, This looks largely OK to me now, though I still have a couple comments. On 09/29/2017 11:58 PM, Sergio Durigan Junior wrote: > @@ -2434,19 +2435,43 @@ variables to files that are only run when you sign on, such as > @section Your Program's Working Directory > > @cindex working directory (of your program) > -Each time you start your program with @code{run}, it inherits its > -working directory from the current working directory of @value{GDBN}. > -The @value{GDBN} working directory is initially whatever it inherited > -from its parent process (typically the shell), but you can specify a new > -working directory in @value{GDBN} with the @code{cd} command. > +Each time you start your program with @code{run}, the inferior will be > +initialized with the current working directory specified by the > +@kbd{set cwd} command. If no directory has been specified by this > +command, then the inferior will inherit @value{GDBN}'s current working > +directory as its working directory. > + > +You can also change @value{GDBN}'s current working directory by using > +the @code{cd} command. > > The @value{GDBN} working directory also serves as a default for the commands > that specify files for @value{GDBN} to operate on. @xref{Files, ,Commands to > Specify Files}. Rereading this, I think that this paragraph ("The GDB working directory also serves...") would be better moved below, where "cd" is documented (with the "also" probably dropped). Maybe add an xref here taking users there. > @kindex cd > -@cindex change working directory > +@cindex change @value{GDBN}'s working directory > @item cd @r{[}@var{directory}@r{]} > Set the @value{GDBN} working directory to @var{directory}. If not > given, @var{directory} uses @file{'~'}. Above I meant, move that paragraph here, where "cd" is documented. This is where I'd expect to see info about what does the command affect. Maybe also add an xref to "set cwd" here. > @@ -374,6 +389,14 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs, > UIs. */ > close_most_fds (); > > + /* Change to the requested working directory if the user > + requested it. */ > + if (inferior_cwd != NULL) > + { > + if (chdir (inferior_cwd) < 0) > + trace_start_error_with_name (expanded_inferior_cwd.c_str ()); Please also update the trace_start... statement: trace_start_error_with_name (expanded_inferior); > +proc_with_prefix test_cwd_reset { } { > + global decimal gdb_prompt tmpdir > + > + gdb_test_multiple "pwd" "GDB cwd" { > + -re "Working directory \(.*\)\.\r\n$gdb_prompt $" { > + set gdb_cwd $expect_out(1,string) > + } > + -re ".*$gdb_prompt $" { > + fail "failed to obtain GDB cwd before run" > + return -1 > + } > + default { > + fail "failed to obtain GDB cwd before run" > + return -1 > + } > + } Any reason you didn't update this one? > > + const char *inferior_cwd = get_inferior_cwd (); > + std::string expanded_infcwd; > + if (inferior_cwd != NULL) > + { > + expanded_infcwd = gdb_tilde_expand (inferior_cwd); > + /* Mirror slashes on inferior's cwd. */ > + std::replace (expanded_infcwd.begin (), expanded_infcwd.end (), > + '/', '\\'); > + inferior_cwd = expanded_infcwd.c_str (); > + } > + But what if inferior_cwd _is_ NULL, when ... > memset (&si, 0, sizeof (si)); > si.cb = sizeof (si); > > @@ -2514,6 +2527,10 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file, > flags |= DEBUG_PROCESS; > } > > + if (cygwin_conv_path (CCP_POSIX_TO_WIN_W, inferior_cwd, > + infcwd, strlen (inferior_cwd)) < 0) > + error (_("Error converting inferior cwd: %d"), errno); ... we get here? It looks to me like this conversion should skipped here then, and ... > + > #ifdef __USEWIDE > args = (cygwin_buf_t *) alloca ((wcslen (toexec) + wcslen (cygallargs) + 2) > * sizeof (wchar_t)); > @@ -2574,7 +2591,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file, > TRUE, /* inherit handles */ > flags, /* start flags */ > w32_env, /* environment */ > - NULL, /* current directory */ > + infcwd, /* current directory */ ... here still pass NULL. > &si, > &pi); > if (w32_env) > @@ -2697,7 +2714,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file, > TRUE, /* inherit handles */ > flags, /* start flags */ > w32env, /* environment */ > - NULL, /* current directory */ > + inferior_cwd, /* current directory */ > &si, > &pi); > if (tty != INVALID_HANDLE_VALUE) > Thanks, Pedro Alves