* [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore consecutive '\'
@ 2017-05-12 16:00 Jeff Westfahl
2017-05-16 7:48 ` Ni, Ruiyu
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Westfahl @ 2017-05-12 16:00 UTC (permalink / raw)
To: edk2-devel
Cc: Jeff Westfahl, Michael D Kinney, Liming Gao, Ruiyu Ni,
Jaben Carsey
This patch makes PathRemoveLastItem ignore consecutive occurrences of the
'\' path separator.
Consider a path like "FS0:\ABC\DEF\\", noting the consecutive '\' path
separator characters at the end. The expected result of PathRemoveLastItem
on such a path is "FS0:\ABC\". However, what we get is "FS0:\ABC\DEF\".
We can see the result of this behavior with 'ls' in the EFI shell. Go a
couple of folders deep into a filesystem and try 'ls ..\..'. Here's an
example, with a filesystem with folder ABC in the root, with subfolder
DEF.
FS0:\ABC\DEF\> ls ..
Directory of: FS0:\ABC\
05/12/2017 15:46 <DIR> 8,192 .
05/12/2017 15:46 <DIR> 0 ..
05/12/2017 15:46 <DIR> 8,192 DEF
0 File(s) 0 bytes
3 Dir(s)
FS0:\ABC\DEF\> ls ..\..
Directory of: FS0:\ABC\
05/12/2017 15:46 <DIR> 8,192 .
05/12/2017 15:46 <DIR> 0 ..
05/12/2017 15:46 <DIR> 8,192 DEF
0 File(s) 0 bytes
3 Dir(s)
fs0:\ABC\DEF\>
As you can see, 'ls ..\..' lists only the parent folder. This patch
resolves the issue so that 'ls ..\..' lists the grandparent folder.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
---
MdePkg/Library/BaseLib/FilePaths.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/MdePkg/Library/BaseLib/FilePaths.c b/MdePkg/Library/BaseLib/FilePaths.c
index 203045c..bbaf140 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -37,9 +37,7 @@ PathRemoveLastItem(
; Walker != NULL && *Walker != CHAR_NULL
; Walker++
){
- if (*Walker == L'\\' && *(Walker + 1) != CHAR_NULL) {
- LastSlash = Walker+1;
- } else if (*Walker == L':' && *(Walker + 1) != L'\\' && *(Walker + 1) != CHAR_NULL) {
+ if ((*Walker == L'\\' || *Walker == L':') && *(Walker + 1) != L'\\' && *(Walker + 1) != CHAR_NULL) {
LastSlash = Walker+1;
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore consecutive '\'
2017-05-12 16:00 [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore consecutive '\' Jeff Westfahl
@ 2017-05-16 7:48 ` Ni, Ruiyu
2017-05-16 16:20 ` Jeff Westfahl
0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ruiyu @ 2017-05-16 7:48 UTC (permalink / raw)
To: Jeff Westfahl, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Carsey, Jaben, Gao, Liming
Jeff,
PathRemoveLastItem() is expected to be called after PathCleanUpDirectories().
E.g.: what should we expect PathRemoveLastItem() do for "fs0:\a\b\..\"?
So PathRemoveLastItem() expects the incoming path is cleaned.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jeff Westfahl
> Sent: Saturday, May 13, 2017 12:01 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2] [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore
> consecutive '\'
>
> This patch makes PathRemoveLastItem ignore consecutive occurrences of
> the '\' path separator.
>
> Consider a path like "FS0:\ABC\DEF\\", noting the consecutive '\' path
> separator characters at the end. The expected result of
> PathRemoveLastItem on such a path is "FS0:\ABC\". However, what we get is
> "FS0:\ABC\DEF\".
>
> We can see the result of this behavior with 'ls' in the EFI shell. Go a couple of
> folders deep into a filesystem and try 'ls ..\..'. Here's an example, with a
> filesystem with folder ABC in the root, with subfolder DEF.
>
> FS0:\ABC\DEF\> ls ..
> Directory of: FS0:\ABC\
> 05/12/2017 15:46 <DIR> 8,192 .
> 05/12/2017 15:46 <DIR> 0 ..
> 05/12/2017 15:46 <DIR> 8,192 DEF
> 0 File(s) 0 bytes
> 3 Dir(s)
> FS0:\ABC\DEF\> ls ..\..
> Directory of: FS0:\ABC\
> 05/12/2017 15:46 <DIR> 8,192 .
> 05/12/2017 15:46 <DIR> 0 ..
> 05/12/2017 15:46 <DIR> 8,192 DEF
> 0 File(s) 0 bytes
> 3 Dir(s)
> fs0:\ABC\DEF\>
>
> As you can see, 'ls ..\..' lists only the parent folder. This patch resolves the
> issue so that 'ls ..\..' lists the grandparent folder.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
> ---
> MdePkg/Library/BaseLib/FilePaths.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/FilePaths.c
> b/MdePkg/Library/BaseLib/FilePaths.c
> index 203045c..bbaf140 100644
> --- a/MdePkg/Library/BaseLib/FilePaths.c
> +++ b/MdePkg/Library/BaseLib/FilePaths.c
> @@ -37,9 +37,7 @@ PathRemoveLastItem(
> ; Walker != NULL && *Walker != CHAR_NULL
> ; Walker++
> ){
> - if (*Walker == L'\\' && *(Walker + 1) != CHAR_NULL) {
> - LastSlash = Walker+1;
> - } else if (*Walker == L':' && *(Walker + 1) != L'\\' && *(Walker + 1) !=
> CHAR_NULL) {
> + if ((*Walker == L'\\' || *Walker == L':') && *(Walker + 1) != L'\\'
> + && *(Walker + 1) != CHAR_NULL) {
> LastSlash = Walker+1;
> }
> }
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore consecutive '\'
2017-05-16 7:48 ` Ni, Ruiyu
@ 2017-05-16 16:20 ` Jeff Westfahl
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Westfahl @ 2017-05-16 16:20 UTC (permalink / raw)
To: Ni, Ruiyu
Cc: Jeff Westfahl, edk2-devel@lists.01.org, Kinney, Michael D,
Carsey, Jaben, Gao, Liming
Hi Ray,
The problem with 'ls ..\..\' that I'm trying to solve here is the call to
PathRemoveLastItem from within PathCleanUpDirectories. This intermediate
path is not clean. Here is a log of the operations performed on a path as
it goes through PathCleanUpDirectories:
FS2:\ABC\DEF\> ls ..\..
ShellCommandRunLs: FullPath FS2:\ABC\DEF\..\..
ShellCommandRunLs: PathName ..\..
PathCleanUpDirectories: Entry: FS2:\ABC\DEF\..\..\*
PathCleanUpDirectories: Stp4a: FS2:\ABC\DEF\
PathRemoveLastItem: Entry FS2:\ABC\DEF\
PathRemoveLastItem: Exit1 FS2:\ABC\
PathCleanUpDirectories: Stp4b: FS2:\ABC\
PathCleanUpDirectories: Stp4c: FS2:\ABC\\..\*
PathCleanUpDirectories: Stp4a: FS2:\ABC\\
PathRemoveLastItem: Entry FS2:\ABC\\
PathRemoveLastItem: Exit1 FS2:\ABC\
PathCleanUpDirectories: Stp4b: FS2:\ABC\
PathCleanUpDirectories: Stp4c: FS2:\ABC\\*
PathCleanUpDirectories: Step5: FS2:\ABC\*
PathCleanUpDirectories: Exit: FS2:\ABC\*
As you can see, on the second iteration through the while loop (Step 4) on
PathCleanUpDirectories that removes all of the "\..", we pass an unclean
path to PathRemoveLastItem.
It's difficult to fix this problem in PathCleanUpDirectories because the
unclean path can be, well, unclean. For example, "FS2:\ABC\DEF\..\\\\..".
Please let me know what you think.
Regards,
Jeff
On Tue, 16 May 2017, Ni, Ruiyu wrote:
> Jeff,
> PathRemoveLastItem() is expected to be called after PathCleanUpDirectories().
> E.g.: what should we expect PathRemoveLastItem() do for "fs0:\a\b\..\"?
> So PathRemoveLastItem() expects the incoming path is cleaned.
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Jeff Westfahl
>> Sent: Saturday, May 13, 2017 12:01 AM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Gao, Liming
>> <liming.gao@intel.com>
>> Subject: [edk2] [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore
>> consecutive '\'
>>
>> This patch makes PathRemoveLastItem ignore consecutive occurrences of
>> the '\' path separator.
>>
>> Consider a path like "FS0:\ABC\DEF\\", noting the consecutive '\' path
>> separator characters at the end. The expected result of
>> PathRemoveLastItem on such a path is "FS0:\ABC\". However, what we get is
>> "FS0:\ABC\DEF\".
>>
>> We can see the result of this behavior with 'ls' in the EFI shell. Go a couple of
>> folders deep into a filesystem and try 'ls ..\..'. Here's an example, with a
>> filesystem with folder ABC in the root, with subfolder DEF.
>>
>> FS0:\ABC\DEF\> ls ..
>> Directory of: FS0:\ABC\
>> 05/12/2017 15:46 <DIR> 8,192 .
>> 05/12/2017 15:46 <DIR> 0 ..
>> 05/12/2017 15:46 <DIR> 8,192 DEF
>> 0 File(s) 0 bytes
>> 3 Dir(s)
>> FS0:\ABC\DEF\> ls ..\..
>> Directory of: FS0:\ABC\
>> 05/12/2017 15:46 <DIR> 8,192 .
>> 05/12/2017 15:46 <DIR> 0 ..
>> 05/12/2017 15:46 <DIR> 8,192 DEF
>> 0 File(s) 0 bytes
>> 3 Dir(s)
>> fs0:\ABC\DEF\>
>>
>> As you can see, 'ls ..\..' lists only the parent folder. This patch resolves the
>> issue so that 'ls ..\..' lists the grandparent folder.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
>> ---
>> MdePkg/Library/BaseLib/FilePaths.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/MdePkg/Library/BaseLib/FilePaths.c
>> b/MdePkg/Library/BaseLib/FilePaths.c
>> index 203045c..bbaf140 100644
>> --- a/MdePkg/Library/BaseLib/FilePaths.c
>> +++ b/MdePkg/Library/BaseLib/FilePaths.c
>> @@ -37,9 +37,7 @@ PathRemoveLastItem(
>> ; Walker != NULL && *Walker != CHAR_NULL
>> ; Walker++
>> ){
>> - if (*Walker == L'\\' && *(Walker + 1) != CHAR_NULL) {
>> - LastSlash = Walker+1;
>> - } else if (*Walker == L':' && *(Walker + 1) != L'\\' && *(Walker + 1) !=
>> CHAR_NULL) {
>> + if ((*Walker == L'\\' || *Walker == L':') && *(Walker + 1) != L'\\'
>> + && *(Walker + 1) != CHAR_NULL) {
>> LastSlash = Walker+1;
>> }
>> }
>> --
>> 2.7.4
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-16 16:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-12 16:00 [PATCH] MdePkg/BaseLib: Fix PathRemoveLastItem to ignore consecutive '\' Jeff Westfahl
2017-05-16 7:48 ` Ni, Ruiyu
2017-05-16 16:20 ` Jeff Westfahl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox