Pushed. Both patches. Thanks/Ray
> -----Original Message----- > From: jim.dai...@dell.com <jim.dai...@dell.com> > Sent: Wednesday, October 31, 2018 7:28 PM > To: Ni, Ruiyu <ruiyu...@intel.com> > Cc: Carsey, Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org > Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully- > qualify paths > > 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. > > 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)! > > Regards, > Jim > > -----Original Message----- > From: Ni, Ruiyu [mailto:ruiyu...@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 fully- > qualify paths > > 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 > system. > // > if (InputPath == NULL) { > InputPath = L""; > } > > If yes, I can remove it for you and push the patch. > > Thanks/Ray > > > -----Original Message----- > > From: jim.dai...@dell.com <jim.dai...@dell.com> > > Sent: Tuesday, October 30, 2018 7:32 PM > > To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org > > Cc: Carsey, Jaben <jaben.car...@intel.com> > > Subject: RE: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to > > fully- qualify paths > > > > >-----Original Message----- > > >From: Ni, Ruiyu [mailto:ruiyu...@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.dai...@dell.com <jim.dai...@dell.com> > > >> Sent: Tuesday, October 30, 2018 5:15 AM > > >> To: edk2-devel@lists.01.org > > >> Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ruiyu > > >> <ruiyu...@intel.com> > > >> 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 = NULL; > > >> + > > >> + ASSERT(Path != NULL); > > >> + // > > >> + // Handle erroneous input when asserts are disabled. > > >> + // > > >> + if (Path == 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 = StrStr(Path, L":"); > > >> + if (InputPath != NULL) { > > >> + InputPath++; > > >> + FileSystemCharCount = ((UINTN)InputPath - (UINTN)Path + > > >> sizeof(CHAR16)) / sizeof(CHAR16); > > >> + InputFileSystem = AllocateCopyPool(FileSystemCharCount * > > >> sizeof(CHAR16), Path); > > >> + if (InputFileSystem != NULL) { > > >> + InputFileSystem[FileSystemCharCount - 1] = CHAR_NULL; > > >> + } > > >> + WorkingPath = 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 == 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
