Re: [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully-qualify paths

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Pushed. Both patches.

Thanks/Ray

> -----Original Message-----
> From: Jim.Dailey@dell.com <Jim.Dailey@dell.com>
> Sent: Wednesday, October 31, 2018 7:28 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@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.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 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.Dailey@dell.com <Jim.Dailey@dell.com>
> > Sent: Tuesday, October 30, 2018 7:32 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>
> > 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 <Jim.Dailey@dell.com>
> > >> Sent: Tuesday, October 30, 2018 5:15 AM
> > >> To: edk2-devel@lists.01.org
> > >> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu
> > >> <ruiyu.ni@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