From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E657B21B02822 for ; Thu, 4 Oct 2018 10:07:25 -0700 (PDT) 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 71DAA3002F4F; Thu, 4 Oct 2018 17:07:25 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-104.rdu2.redhat.com [10.10.120.104]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5FA8F67C7F; Thu, 4 Oct 2018 17:07:24 +0000 (UTC) To: "Carsey, Jaben" , "Jim.Dailey@dell.com" , "edk2-devel@lists.01.org" References: From: Laszlo Ersek Message-ID: Date: Thu, 4 Oct 2018 19:07:23 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Thu, 04 Oct 2018 17:07:25 +0000 (UTC) Subject: Re: [PATCH] ShellPkg: Create a homefilesystem environment variable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Oct 2018 17:07:26 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/03/18 20:17, Carsey, Jaben wrote: > Pushed. > c0b1f749ef1304810ed4ea58ded65b7f41d79d3e Please give other reviewers a bit more time than ~2 hours, to comment on the patch. :) I think I would have suggested an improvement (or a clarification about) the commit message. It says: >> -----Original Message----- >> From: Carsey, Jaben >> Sent: Wednesday, October 03, 2018 11:15 AM >> To: 'Jim.Dailey@dell.com' ; edk2-devel@lists.01.org >> Subject: RE: [edk2] [PATCH] ShellPkg: Create a homefilesystem environment >> variable >> >> Reviewed-by: Jaben Carsey >> >>> -----Original Message----- >>> From: Jim.Dailey@dell.com [mailto:Jim.Dailey@dell.com] >>> Sent: Wednesday, October 03, 2018 9:02 AM >>> To: edk2-devel@lists.01.org >>> Cc: Carsey, Jaben ; Ni, Ruiyu >> >>> Subject: [edk2] [PATCH] ShellPkg: Create a homefilesystem environment >>> variable >>> Importance: High >>> >>> Create a homefilesystem environment variable whose value is the file >>> system on which the executing shell is located. For example: "FS14:". that the file system in question contains the *shell*. So my first question would have been, what if the shell is memory mapped (from a firmware volume), but the platform doesn't expose firmware filesystems (FFSs) as EFI simple file system protocol instances? In that case, the "file system on which the executing shell is located" seems ill-defined. >>> >>> This eliminates the need for people to have to try and find the "boot" >>> file system in their startup script. After this change they can simply Note, here the commit message refers to the startup script, not the shell itself. >>> execute %homefilesystem% to set the cwd to the root of the file system >>> where the shell is located. I think the commit message here misses a "CD" command. >>> >>> A future enhancement could be to add "homefilesystem" to the list of >>> predefined, read-only variables listed in the EfiShellSetEnv function of >>> file ShellProtocol.c Is it OK with the UEFI shell spec to define a shell variable called "homefilesystem"? I seem to remember that edk2-specific options for standard UEFI shell commands generally start with an underscore, to avoid clashing with the standard namespace. Does that apply to shell variables perhaps? (This is mostly for my own education.) >>> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Jim Dailey >>> --- >>> ShellPkg/Application/Shell/Shell.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/ShellPkg/Application/Shell/Shell.c >>> b/ShellPkg/Application/Shell/Shell.c >>> index 3f3bcbb4b0..6185b6ac80 100644 >>> --- a/ShellPkg/Application/Shell/Shell.c >>> +++ b/ShellPkg/Application/Shell/Shell.c >>> @@ -1169,6 +1169,8 @@ LocateStartupScript ( >>> *TempSpot = CHAR_NULL; >>> } >>> >>> + InternalEfiShellSetEnv(L"homefilesystem", StartupScriptPath, TRUE); >>> + Again, this refers to the startup script, not the shell itself. >>> StartupScriptPath = StrnCatGrow (&StartupScriptPath, &Size, >>> ((FILEPATH_DEVICE_PATH *)FileDevicePath)->PathName, 0); >>> PathRemoveLastItem (StartupScriptPath); >>> StartupScriptPath = StrnCatGrow (&StartupScriptPath, &Size, >>> mStartupScript, 0); >>> -- >>> 2.17.0.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > Thanks Laszlo