Re: [edk2] [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully-qualify paths
Pushed. Both patches.
Thanks/Ray
> -Original Message-
> From: jim.dai...@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 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
> > 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...@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
> > >> 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;
> > >> + UINTNFileSystemCharCount;
> > >> + CHAR16 *FullyQualifiedPath;
> > >> + UINTNSize;
> > >> +
> > >> + 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
Re: [edk2] [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
> 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...@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
> >> 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;
> >> + UINTNFileSystemCharCount;
> >> + CHAR16 *FullyQualifiedPath;
> >> + UINTNSize;
> >> +
> >> + 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
Re: [edk2] [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
> 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...@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
> >> 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;
> >> + UINTNFileSystemCharCount;
> >> + CHAR16 *FullyQualifiedPath;
> >> + UINTNSize;
> >> +
> >> + 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
Re: [edk2] [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
>> 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;
>> + UINTNFileSystemCharCount;
>> + CHAR16 *FullyQualifiedPath;
>> + UINTNSize;
>> +
>> + 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
Re: [edk2] [PATCH v2 1/2] ShellPkg-UefiShellLib: Add a function to fully-qualify paths
> -Original Message-
> From: jim.dai...@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;
> + UINTNFileSystemCharCount;
> + CHAR16 *FullyQualifiedPath;
> + UINTNSize;
> +
> + 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.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
