From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (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 5248121B02822 for ; Sun, 4 Nov 2018 18:59:39 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2018 18:59:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,466,1534834800"; d="scan'208";a="277089835" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 04 Nov 2018 18:59:38 -0800 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 4 Nov 2018 18:59:38 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 4 Nov 2018 18:59:38 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.117]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.161]) with mapi id 14.03.0415.000; Mon, 5 Nov 2018 10:59:25 +0800 From: "Ni, Ruiyu" To: "Jim.Dailey@dell.com" CC: "Carsey, Jaben" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully-qualify paths Thread-Index: AdRvyvFMWA6pVzosSjCUose/JKCJhgAV+iZAAAgH8YAALdzFMAAEbJsQAOnmuNA= Date: Mon, 5 Nov 2018 02:59:24 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BEF0ED7@SHSMSX104.ccr.corp.intel.com> References: <517a9546603441cc868554cb350e8afe@ausx13mps335.AMER.DELL.COM> <734D49CCEBEEF84792F5B80ED585239D5BEE552B@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5BEE8E87@SHSMSX104.ccr.corp.intel.com> <063b9052d04648e2890f0f95232929df@ausx13mps335.AMER.DELL.COM> In-Reply-To: <063b9052d04648e2890f0f95232929df@ausx13mps335.AMER.DELL.COM> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully-qualify paths 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: Mon, 05 Nov 2018 02:59:40 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Pushed. Both patches. Thanks/Ray > -----Original Message----- > From: Jim.Dailey@dell.com > Sent: Wednesday, October 31, 2018 7:28 PM > To: Ni, Ruiyu > Cc: Carsey, Jaben ; edk2-devel@lists.01.org > Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to full= y- > qualify paths >=20 > Yes, Ray, that is the code block that I meant. It serves no purpose since > InputPath cannot possibly be NULL in that part of the code. >=20 > I appreciate your help in deleting this block and pushing the rest of the= patch. > Also, thanks for your thorough review of this code (and the first version= )! >=20 > Regards, > Jim >=20 > -----Original Message----- > From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] > Sent: Wednesday, October 31, 2018 4:18 AM > To: Dailey, Jim; edk2-devel@lists.01.org > Cc: Carsey, Jaben > Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to full= y- > qualify paths >=20 > Jim, > I checked the other parts in your patch. They looks good. > But I don't quite understand how to handle the if-statement. > Do you mean to remove the below code block? > // > // Handle the degenerate case where Path was only a file system > reference. > // In that case we return the current working directory of the file s= ystem. > // > if (InputPath =3D=3D NULL) { > InputPath =3D L""; > } >=20 > If yes, I can remove it for you and push the patch. >=20 > Thanks/Ray >=20 > > -----Original Message----- > > From: Jim.Dailey@dell.com > > Sent: Tuesday, October 30, 2018 7:32 PM > > To: Ni, Ruiyu ; edk2-devel@lists.01.org > > Cc: Carsey, Jaben > > Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to > > fully- qualify paths > > > > >-----Original Message----- > > >From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] > > >Sent: Tuesday, October 30, 2018 2:33 AM > > >To: Dailey, Jim; edk2-devel@lists.01.org > > >Cc: Carsey, Jaben > > >Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to > > >fully-qualify paths > > > > > > > > >> -----Original Message----- > > >> From: Jim.Dailey@dell.com > > >> Sent: Tuesday, October 30, 2018 5:15 AM > > >> To: edk2-devel@lists.01.org > > >> Cc: Carsey, Jaben ; Ni, Ruiyu > > >> > > >> Subject: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to > > >> fully-qualify paths > > >> > > >> +CHAR16* > > >> +EFIAPI > > >> +FullyQualifyPath( > > >> + IN CONST CHAR16 *Path > > >> + ) > > >> +{ > > >> + CONST CHAR16 *WorkingPath; > > >> + CONST CHAR16 *InputPath; > > >> + CHAR16 *InputFileSystem; > > >> + UINTN FileSystemCharCount; > > >> + CHAR16 *FullyQualifiedPath; > > >> + UINTN Size; > > >> + > > >> + FullyQualifiedPath =3D NULL; > > >> + > > >> + ASSERT(Path !=3D NULL); > > >> + // > > >> + // Handle erroneous input when asserts are disabled. > > >> + // > > >> + if (Path =3D=3D NULL) { > > >> + return NULL; > > >> + } > > >> + // > > >> + // In paths that contain ":", like fs0:dir/file.ext and > > >> + fs2:\fqpath\file.ext, // we have to consider the file system > > >> + part > > separately from the "path" > > >> part. > > >> + // If there is a file system in the path, we have to get the > > >> + current working // directory for that file system. Then we need > > >> + to use the part of the path // following the ":". If a path > > >> + does not contain > > ":", we use it as given. > > >> + // > > >> + InputPath =3D StrStr(Path, L":"); > > >> + if (InputPath !=3D NULL) { > > >> + InputPath++; > > >> + FileSystemCharCount =3D ((UINTN)InputPath - (UINTN)Path + > > >> sizeof(CHAR16)) / sizeof(CHAR16); > > >> + InputFileSystem =3D AllocateCopyPool(FileSystemCharCount * > > >> sizeof(CHAR16), Path); > > >> + if (InputFileSystem !=3D NULL) { > > >> + InputFileSystem[FileSystemCharCount - 1] =3D CHAR_NULL; > > >> + } > > >> + WorkingPath =3D ShellGetCurrentDir(InputFileSystem); > > >> + SHELL_FREE_NON_NULL(InputFileSystem); > > >> + // > > >> + // Handle the degenerate case where Path was only a file > > >> + system > > >> reference. > > >> + // In that case we return the current working directory of the > > >> + file > > system. > > >> + // > > >> + if (InputPath =3D=3D NULL) { > > > > > >The "InputPath" should not be NULL. > > > > You are correct. It will simply point to an empty string if the input > > path is only a file system reference (e.g. "FS0:"). I thoughtlessly > > confused an empty string with NULL. :-( > > > > Do you want me to delete that comment and the "if" and resubmit, or, > > assuming the rest of the patch is acceptable, do you want to delete it > > and push the modified patch? > > > > Regards, > > Jim