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