[PATCH] ShellPkg/Ls: Consider UEFI timezone may not be set

[PATCH] ShellPkg/Ls: Consider UEFI timezone may not be set

[Marvin Häuser]

From: Marvin Haeuser <mhaeuser@outlook.de>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2294

EFI_RUNTIME_SERVICES.GetTime() might return an unspecified Timezone,
such as when SetTime() has not been called after the RTC was cut off
power. Consider this case by not attempting Timezone translations for
when it is invalid.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
---
 ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
index adeb987e6ecb..1a65f60c3b44 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
@@ -500,7 +500,7 @@ PrintLsOutput(
       // Change the file time to local time.
       //
       Status = gRT->GetTime(&LocalTime, NULL);
-      if (!EFI_ERROR (Status)) {
+      if (!EFI_ERROR (Status) && (LocalTime.TimeZone != EFI_UNSPECIFIED_TIMEZONE)) {
         if ((Node->Info->CreateTime.TimeZone != EFI_UNSPECIFIED_TIMEZONE) &&
             (Node->Info->CreateTime.Month >= 1 && Node->Info->CreateTime.Month <= 12)) {
           //
-- 
2.23.0.windows.1

[PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

[Marvin Häuser]

From: Marvin Haeuser <mhaeuser@outlook.de>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295

The current implementation of the FileHandleGetFileName() function
assumes that the Root directory always has the FileName '\0'.
However, the only requirement the UEFI specification defines is that
a prepended '\\' must be supported to access files and folders
relative to the Root directory.
This patch removes this assumption and supports constructing valid
paths for any value of FileName for the Root Directory.

In practice, this fixes compatibility issues with File System drivers
that report '\\' as the FileName of the Root directory, which
currently is both generating an invalid path ("\\\\") and resulting
in an EFI_NOT_FOUND result from the CurrentHandle->Open() call.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
---
 MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25 ++++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
index 5dc893833a46..28e28e5f67d5 100644
--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
@@ -816,10 +816,25 @@ FileHandleGetFileName (
         Status = EFI_OUT_OF_RESOURCES;
         break;
       } else {
+        //
+        // Prepare to move to the parent directory.
+        // Also determine whether CurrentHandle refers to the Root directory.
+        //
+        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle, L"..", EFI_FILE_MODE_READ, 0);
         //
         // We got info... do we have a name? if yes precede the current path with it...
         //
-        if (StrLen (FileInfo->FileName) == 0) {
+        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
+          //
+          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
+          // CurrentHandle refers to the Root directory.  As this loop ensures
+          // FullFileName is starting with '\\' at all times, signal success
+          // and exit the loop.
+          // While FileInfo->FileName could theoretically be a value other than
+          // '\0' or '\\', '\\' is guaranteed to be supported by the
+          // specification and hence its value can safely be ignored.
+          //
+          Status = EFI_SUCCESS;
           if (*FullFileName == NULL) {
             ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName != NULL));
             *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0);
@@ -837,15 +852,11 @@ FileHandleGetFileName (
           FreePool(FileInfo);
         }
       }
+
+      FileHandleClose(CurrentHandle);
       //
       // Move to the parent directory
       //
-      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle, L"..", EFI_FILE_MODE_READ, 0);
-      if (EFI_ERROR (Status)) {
-        break;
-      }
-
-      FileHandleClose(CurrentHandle);
       CurrentHandle = NextHigherHandle;
     }
   } else if (Status == EFI_NOT_FOUND) {
-- 
2.23.0.windows.1

[PATCH] ShellPkg/Ls: Return empty content for all empty folders

[Marvin Häuser]

From: Marvin Haeuser <mhaeuser@outlook.de>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2296

Currently, when 'ls' is run on an entirely empty directory (this
includes not having '.' and '..'), the output is always 'File not
found'. For when not filtering its children, this patch rather
displays the usual header and footer.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
---
 ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 33 +++++++++++++++++---
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
index 1a65f60c3b44..da2b1acab47c 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
@@ -417,6 +417,8 @@ FileTimeToLocalTime (
   @param[in] Found          Set to TRUE, if anyone were found.
   @param[in] Count          The count of bits enabled in Attribs.
   @param[in] TimeZone       The current time zone offset.
+  @param[in] ListUnfiltered TRUE to request listing the directory contents
+                            unfiltered.
 
   @retval SHELL_SUCCESS     the printing was sucessful.
 **/
@@ -429,7 +431,8 @@ PrintLsOutput(
   IN CONST CHAR16  *SearchString,
   IN       BOOLEAN *Found,
   IN CONST UINTN   Count,
-  IN CONST INT16   TimeZone
+  IN CONST INT16   TimeZone,
+  IN CONST BOOLEAN ListUnfiltered
   )
 {
   EFI_STATUS            Status;
@@ -555,7 +558,7 @@ PrintLsOutput(
       HeaderPrinted = TRUE;
     }
 
-    if (!Sfo && ShellStatus != SHELL_ABORTED) {
+    if (!Sfo && ShellStatus != SHELL_ABORTED && HeaderPrinted) {
       PrintNonSfoFooter(FileCount, FileSize, DirCount);
     }
   }
@@ -602,7 +605,8 @@ PrintLsOutput(
             SearchString,
             &FoundOne,
             Count,
-            TimeZone);
+            TimeZone,
+            FALSE);
 
           //
           // Since it's running recursively, we have to break immediately when returned SHELL_ABORTED
@@ -619,7 +623,21 @@ PrintLsOutput(
   ShellCloseFileMetaArg(&ListHead);
 
   if (Found == NULL && !FoundOne) {
-    return (SHELL_NOT_FOUND);
+    if (ListUnfiltered) {
+      //
+      // When running "ls" without any filtering request, avoid outputing
+      // "File not found" when the directory is entirely empty, but print
+      // header and footer stating "0 File(s), 0 Dir(s)".
+      //
+      if (!Sfo) {
+        PrintNonSfoHeader (RootPath);
+        if (ShellStatus != SHELL_ABORTED) {
+          PrintNonSfoFooter (FileCount, FileSize, DirCount);
+        }
+      }
+    } else {
+      return (SHELL_NOT_FOUND);
+    }
   }
 
   if (Found != NULL) {
@@ -662,6 +680,7 @@ ShellCommandRunLs (
   UINTN         Size;
   EFI_TIME      TheTime;
   CHAR16        *SearchString;
+  BOOLEAN       ListUnfiltered;
 
   Size                = 0;
   FullPath            = NULL;
@@ -673,6 +692,7 @@ ShellCommandRunLs (
   SearchString        = NULL;
   CurDir              = NULL;
   Count               = 0;
+  ListUnfiltered      = FALSE;
 
   //
   // initialize the shell lib (we must be in non-auto-init...)
@@ -768,6 +788,7 @@ ShellCommandRunLs (
             ShellStatus = SHELL_NOT_FOUND;
             ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_CWD), gShellLevel2HiiHandle, L"ls");
           }
+          ListUnfiltered = TRUE;
           //
           // Copy to the 2 strings for starting path and file search string
           //
@@ -808,6 +829,7 @@ ShellCommandRunLs (
               //
               // is listing ends with a directory, then we list all files in that directory
               //
+              ListUnfiltered = TRUE;
               StrnCatGrow(&SearchString, NULL, L"*", 0);
             } else {
               //
@@ -839,7 +861,8 @@ ShellCommandRunLs (
             SearchString,
             NULL,
             Count,
-            TheTime.TimeZone
+            TheTime.TimeZone,
+            ListUnfiltered
            );
           if (ShellStatus == SHELL_NOT_FOUND) {
             ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_LS_FILE_NOT_FOUND), gShellLevel2HiiHandle, L"ls", FullPath);
-- 
2.23.0.windows.1

[PATCH] UefiShellCommandLib: Default to first found UC for unsupported PlatformLang

[Marvin Häuser]

From: Marvin Haeuser <mhaeuser@outlook.de>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2300

On some firmwares PlatformLang is set to the local language (e.g. ru-RU),
however there is no Unicode Collation protocol instance that supports it.
As for missing PlatformLang, fall back to the first found instance.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
---
 ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
index 4c48b65fbc1d..345808a1eac6 100644
--- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
+++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
@@ -107,9 +107,13 @@ CommandInit(
 
       //
       // Without clue provided use the first Unicode Collation2 protocol.
+      // This may happen when PlatformLang is NULL or when no installed Unicode
+      // Collation2 protocol instance supports PlatformLang.
       //
-      if (PlatformLang == NULL) {
+      if (gUnicodeCollation == NULL) {
         gUnicodeCollation = Uc;
+      }
+      if (PlatformLang == NULL) {
         break;
       }
 
-- 
2.23.0.windows.1

[PATCH] MdePkg/UefiDebugLibConOut: Pass the correct buffer size

[Marvin Häuser]

From: Marvin Haeuser <mhaeuser@outlook.de>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2302

The second argument of "UnicodeVSPrintAsciiFormat" is "BufferSize",
which takes the size of the buffer in bytes. Replace the currently
used MAX_DEBUG_MESSAGE_LENGTH usage, which is the buffer's length,
with the actual buffer size.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
---
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
index cf168d05cf21..8ea38ea7cc7c 100644
--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
@@ -104,9 +104,9 @@ DebugPrintMarker (
     // Convert the DEBUG() message to a Unicode String
     //
     if (BaseListMarker == NULL) {
-      UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, VaListMarker);
+      UnicodeVSPrintAsciiFormat (Buffer, sizeof (Buffer), Format, VaListMarker);
     } else {
-      UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, BaseListMarker);
+      UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format, BaseListMarker);
     }
 
 
-- 
2.23.0.windows.1

Re: [PATCH] MdePkg/UefiDebugLibConOut: Pass the correct buffer size

[Liming Gao]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
>Sent: Sunday, October 20, 2019 8:09 PM
>To: devel@edk2.groups.io
>Cc: vit9696@protonmail.com; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>Subject: [PATCH] MdePkg/UefiDebugLibConOut: Pass the correct buffer size
>
>From: Marvin Haeuser <mhaeuser@outlook.de>
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2302
>
>The second argument of "UnicodeVSPrintAsciiFormat" is "BufferSize",
>which takes the size of the buffer in bytes. Replace the currently
>used MAX_DEBUG_MESSAGE_LENGTH usage, which is the buffer's length,
>with the actual buffer size.
>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>---
> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>index cf168d05cf21..8ea38ea7cc7c 100644
>--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>@@ -104,9 +104,9 @@ DebugPrintMarker (
>     // Convert the DEBUG() message to a Unicode String
>
>     //
>
>     if (BaseListMarker == NULL) {
>
>-      UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,
>Format, VaListMarker);
>
>+      UnicodeVSPrintAsciiFormat (Buffer, sizeof (Buffer), Format,
>VaListMarker);
>
>     } else {
>
>-      UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,
>Format, BaseListMarker);
>
>+      UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format,
>BaseListMarker);
>
>     }
>
>
>
>
>
>--
>2.23.0.windows.1

Re: [PATCH] ShellPkg/Ls: Consider UEFI timezone may not be set

[Gao, Zhichao]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

> -----Original Message-----
> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> Sent: Sunday, October 20, 2019 8:09 PM
> To: devel@edk2.groups.io
> Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: [PATCH] ShellPkg/Ls: Consider UEFI timezone may not be set
> 
> From: Marvin Haeuser <mhaeuser@outlook.de>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2294
> 
> EFI_RUNTIME_SERVICES.GetTime() might return an unspecified Timezone,
> such as when SetTime() has not been called after the RTC was cut off power.
> Consider this case by not attempting Timezone translations for when it is
> invalid.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> ---
>  ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> index adeb987e6ecb..1a65f60c3b44 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> @@ -500,7 +500,7 @@ PrintLsOutput(
>        // Change the file time to local time.       //       Status = gRT-
> >GetTime(&LocalTime, NULL);-      if (!EFI_ERROR (Status)) {+      if
> (!EFI_ERROR (Status) && (LocalTime.TimeZone !=
> EFI_UNSPECIFIED_TIMEZONE)) {         if ((Node->Info-
> >CreateTime.TimeZone != EFI_UNSPECIFIED_TIMEZONE) &&             (Node-
> >Info->CreateTime.Month >= 1 && Node->Info->CreateTime.Month <= 12))
> {           //--
> 2.23.0.windows.1

Re: [PATCH] UefiShellCommandLib: Default to first found UC for unsupported PlatformLang

[Gao, Zhichao]

Hi Ray,

This patch would set the default language of shell to the first found language instead of ASSERT when the matched language is not found. What do you think of this change? I don't know the reason of assert. If it is required to ASSERT to show the user the shell language should be matched with the platform language. Then the patch is inappropriate. If not, the patch is fine.

Thanks,
Zhichao

> -----Original Message-----
> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> Sent: Sunday, October 20, 2019 8:09 PM
> To: devel@edk2.groups.io
> Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: [PATCH] UefiShellCommandLib: Default to first found UC for
> unsupported PlatformLang
> 
> From: Marvin Haeuser <mhaeuser@outlook.de>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2300
> 
> On some firmwares PlatformLang is set to the local language (e.g. ru-RU),
> however there is no Unicode Collation protocol instance that supports it.
> As for missing PlatformLang, fall back to the first found instance.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> ---
>  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> index 4c48b65fbc1d..345808a1eac6 100644
> --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> @@ -107,9 +107,13 @@ CommandInit(
>         //       // Without clue provided use the first Unicode Collation2 protocol.+
> // This may happen when PlatformLang is NULL or when no installed
> Unicode+      // Collation2 protocol instance supports PlatformLang.       //-      if
> (PlatformLang == NULL) {+      if (gUnicodeCollation == NULL)
> {         gUnicodeCollation = Uc;+      }+      if (PlatformLang == NULL)
> {         break;       } --
> 2.23.0.windows.1

Re: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

[Gao, Zhichao]

This patch makes sense. The patch would not affect the original logic and figure out the failure if the root directory file name is '\\'.
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Marvin Häuser
> Sent: Sunday, October 20, 2019 8:09 PM
> To: devel@edk2.groups.io
> Cc: vit9696@protonmail.com; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
> Root handle FileNames
> 
> From: Marvin Haeuser <mhaeuser@outlook.de>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295
> 
> The current implementation of the FileHandleGetFileName() function
> assumes that the Root directory always has the FileName '\0'.
> However, the only requirement the UEFI specification defines is that a
> prepended '\\' must be supported to access files and folders relative to the
> Root directory.
> This patch removes this assumption and supports constructing valid paths for
> any value of FileName for the Root Directory.
> 
> In practice, this fixes compatibility issues with File System drivers that report
> '\\' as the FileName of the Root directory, which currently is both generating
> an invalid path ("\\\\") and resulting in an EFI_NOT_FOUND result from the
> CurrentHandle->Open() call.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> ---
>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25
> ++++++++++++++------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> index 5dc893833a46..28e28e5f67d5 100644
> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> @@ -816,10 +816,25 @@ FileHandleGetFileName (
>          Status = EFI_OUT_OF_RESOURCES;
>          break;
>        } else {
> +        //
> +        // Prepare to move to the parent directory.
> +        // Also determine whether CurrentHandle refers to the Root directory.
> +        //
> +        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
> + L"..", EFI_FILE_MODE_READ, 0);
>          //
>          // We got info... do we have a name? if yes precede the current path
> with it...
>          //
> -        if (StrLen (FileInfo->FileName) == 0) {
> +        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
> +          //
> +          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
> +          // CurrentHandle refers to the Root directory.  As this loop ensures
> +          // FullFileName is starting with '\\' at all times, signal success
> +          // and exit the loop.
> +          // While FileInfo->FileName could theoretically be a value other than
> +          // '\0' or '\\', '\\' is guaranteed to be supported by the
> +          // specification and hence its value can safely be ignored.
> +          //
> +          Status = EFI_SUCCESS;
>            if (*FullFileName == NULL) {
>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
> NULL));
>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0); @@ -
> 837,15 +852,11 @@ FileHandleGetFileName (
>            FreePool(FileInfo);
>          }
>        }
> +
> +      FileHandleClose(CurrentHandle);
>        //
>        // Move to the parent directory
>        //
> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
> L"..", EFI_FILE_MODE_READ, 0);
> -      if (EFI_ERROR (Status)) {
> -        break;
> -      }
> -
> -      FileHandleClose(CurrentHandle);
>        CurrentHandle = NextHigherHandle;
>      }
>    } else if (Status == EFI_NOT_FOUND) {
> --
> 2.23.0.windows.1
> 
> 
> 

Re: [PATCH] ShellPkg/Ls: Return empty content for all empty folders

[Gao, Zhichao]

Sorry for missing this one.
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

> -----Original Message-----
> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> Sent: Sunday, October 20, 2019 8:09 PM
> To: devel@edk2.groups.io
> Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: [PATCH] ShellPkg/Ls: Return empty content for all empty folders
> 
> From: Marvin Haeuser <mhaeuser@outlook.de>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2296
> 
> Currently, when 'ls' is run on an entirely empty directory (this includes not
> having '.' and '..'), the output is always 'File not found'. For when not filtering
> its children, this patch rather displays the usual header and footer.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> ---
>  ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c | 33
> +++++++++++++++++---
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> index 1a65f60c3b44..da2b1acab47c 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Ls.c
> @@ -417,6 +417,8 @@ FileTimeToLocalTime (
>    @param[in] Found          Set to TRUE, if anyone were found.   @param[in]
> Count          The count of bits enabled in Attribs.   @param[in] TimeZone
> The current time zone offset.+  @param[in] ListUnfiltered TRUE to request
> listing the directory contents+                            unfiltered.    @retval
> SHELL_SUCCESS     the printing was sucessful. **/@@ -429,7 +431,8 @@
> PrintLsOutput(
>    IN CONST CHAR16  *SearchString,   IN       BOOLEAN *Found,   IN CONST
> UINTN   Count,-  IN CONST INT16   TimeZone+  IN CONST INT16   TimeZone,+
> IN CONST BOOLEAN ListUnfiltered   ) {   EFI_STATUS            Status;@@ -555,7
> +558,7 @@ PrintLsOutput(
>        HeaderPrinted = TRUE;     } -    if (!Sfo && ShellStatus != SHELL_ABORTED)
> {+    if (!Sfo && ShellStatus != SHELL_ABORTED && HeaderPrinted)
> {       PrintNonSfoFooter(FileCount, FileSize, DirCount);     }   }@@ -602,7 +605,8
> @@ PrintLsOutput(
>              SearchString,             &FoundOne,             Count,-            TimeZone);+
> TimeZone,+            FALSE);            //           // Since it's running recursively, we
> have to break immediately when returned SHELL_ABORTED@@ -619,7
> +623,21 @@ PrintLsOutput(
>    ShellCloseFileMetaArg(&ListHead);    if (Found == NULL && !FoundOne) {-
> return (SHELL_NOT_FOUND);+    if (ListUnfiltered) {+      //+      // When
> running "ls" without any filtering request, avoid outputing+      // "File not
> found" when the directory is entirely empty, but print+      // header and
> footer stating "0 File(s), 0 Dir(s)".+      //+      if (!Sfo) {+        PrintNonSfoHeader
> (RootPath);+        if (ShellStatus != SHELL_ABORTED) {+
> PrintNonSfoFooter (FileCount, FileSize, DirCount);+        }+      }+    } else {+
> return (SHELL_NOT_FOUND);+    }   }    if (Found != NULL) {@@ -662,6 +680,7
> @@ ShellCommandRunLs (
>    UINTN         Size;   EFI_TIME      TheTime;   CHAR16        *SearchString;+
> BOOLEAN       ListUnfiltered;    Size                = 0;   FullPath            = NULL;@@ -
> 673,6 +692,7 @@ ShellCommandRunLs (
>    SearchString        = NULL;   CurDir              = NULL;   Count               = 0;+
> ListUnfiltered      = FALSE;    //   // initialize the shell lib (we must be in non-
> auto-init...)@@ -768,6 +788,7 @@ ShellCommandRunLs (
>              ShellStatus = SHELL_NOT_FOUND;             ShellPrintHiiEx(-1, -1, NULL,
> STRING_TOKEN (STR_GEN_NO_CWD), gShellLevel2HiiHandle, L"ls");           }+
> ListUnfiltered = TRUE;           //           // Copy to the 2 strings for starting path
> and file search string           //@@ -808,6 +829,7 @@ ShellCommandRunLs (
>                //               // is listing ends with a directory, then we list all files in that
> directory               //+              ListUnfiltered = TRUE;
> StrnCatGrow(&SearchString, NULL, L"*", 0);             } else {               //@@ -839,7
> +861,8 @@ ShellCommandRunLs (
>              SearchString,             NULL,             Count,-            TheTime.TimeZone+
> TheTime.TimeZone,+            ListUnfiltered            );           if (ShellStatus ==
> SHELL_NOT_FOUND) {             ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_LS_FILE_NOT_FOUND), gShellLevel2HiiHandle, L"ls", FullPath);--
> 2.23.0.windows.1

Re: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

[Liming Gao]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Gao, Zhichao
>Sent: Thursday, October 24, 2019 10:52 AM
>To: devel@edk2.groups.io; Marvin.Haeuser@outlook.com
>Cc: vit9696@protonmail.com; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>Subject: RE: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle
>FileNames
>
>This patch makes sense. The patch would not affect the original logic and
>figure out the failure if the root directory file name is '\\'.
>Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Marvin Häuser
>> Sent: Sunday, October 20, 2019 8:09 PM
>> To: devel@edk2.groups.io
>> Cc: vit9696@protonmail.com; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>> Root handle FileNames
>>
>> From: Marvin Haeuser <mhaeuser@outlook.de>
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295
>>
>> The current implementation of the FileHandleGetFileName() function
>> assumes that the Root directory always has the FileName '\0'.
>> However, the only requirement the UEFI specification defines is that a
>> prepended '\\' must be supported to access files and folders relative to the
>> Root directory.
>> This patch removes this assumption and supports constructing valid paths
>for
>> any value of FileName for the Root Directory.
>>
>> In practice, this fixes compatibility issues with File System drivers that report
>> '\\' as the FileName of the Root directory, which currently is both generating
>> an invalid path ("\\\\") and resulting in an EFI_NOT_FOUND result from the
>> CurrentHandle->Open() call.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>> ---
>>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25
>> ++++++++++++++------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> index 5dc893833a46..28e28e5f67d5 100644
>> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> @@ -816,10 +816,25 @@ FileHandleGetFileName (
>>          Status = EFI_OUT_OF_RESOURCES;
>>          break;
>>        } else {
>> +        //
>> +        // Prepare to move to the parent directory.
>> +        // Also determine whether CurrentHandle refers to the Root directory.
>> +        //
>> +        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>> + L"..", EFI_FILE_MODE_READ, 0);
>>          //
>>          // We got info... do we have a name? if yes precede the current path
>> with it...
>>          //
>> -        if (StrLen (FileInfo->FileName) == 0) {
>> +        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
>> +          //
>> +          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
>> +          // CurrentHandle refers to the Root directory.  As this loop ensures
>> +          // FullFileName is starting with '\\' at all times, signal success
>> +          // and exit the loop.
>> +          // While FileInfo->FileName could theoretically be a value other than
>> +          // '\0' or '\\', '\\' is guaranteed to be supported by the
>> +          // specification and hence its value can safely be ignored.
>> +          //
>> +          Status = EFI_SUCCESS;
>>            if (*FullFileName == NULL) {
>>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
>> NULL));
>>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0); @@ -
>> 837,15 +852,11 @@ FileHandleGetFileName (
>>            FreePool(FileInfo);
>>          }
>>        }
>> +
>> +      FileHandleClose(CurrentHandle);
>>        //
>>        // Move to the parent directory
>>        //
>> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>> L"..", EFI_FILE_MODE_READ, 0);
>> -      if (EFI_ERROR (Status)) {
>> -        break;
>> -      }
>> -
>> -      FileHandleClose(CurrentHandle);
>>        CurrentHandle = NextHigherHandle;
>>      }
>>    } else if (Status == EFI_NOT_FOUND) {
>> --
>> 2.23.0.windows.1
>>
>>
>> 

Re: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

[Liming Gao]

Push @ 6407186db9505f101ece4e1571eceed69b9fbdbe

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Monday, November 04, 2019 8:50 AM
>To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io;
>Marvin.Haeuser@outlook.com
>Cc: vit9696@protonmail.com; Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: Re: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>Root handle FileNames
>
>Reviewed-by: Liming Gao <liming.gao@intel.com>
>
>>-----Original Message-----
>>From: Gao, Zhichao
>>Sent: Thursday, October 24, 2019 10:52 AM
>>To: devel@edk2.groups.io; Marvin.Haeuser@outlook.com
>>Cc: vit9696@protonmail.com; Kinney, Michael D
>><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>>Subject: RE: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle
>>FileNames
>>
>>This patch makes sense. The patch would not affect the original logic and
>>figure out the failure if the root directory file name is '\\'.
>>Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>> Marvin Häuser
>>> Sent: Sunday, October 20, 2019 8:09 PM
>>> To: devel@edk2.groups.io
>>> Cc: vit9696@protonmail.com; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>>> Subject: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>>> Root handle FileNames
>>>
>>> From: Marvin Haeuser <mhaeuser@outlook.de>
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295
>>>
>>> The current implementation of the FileHandleGetFileName() function
>>> assumes that the Root directory always has the FileName '\0'.
>>> However, the only requirement the UEFI specification defines is that a
>>> prepended '\\' must be supported to access files and folders relative to the
>>> Root directory.
>>> This patch removes this assumption and supports constructing valid paths
>>for
>>> any value of FileName for the Root Directory.
>>>
>>> In practice, this fixes compatibility issues with File System drivers that
>report
>>> '\\' as the FileName of the Root directory, which currently is both
>generating
>>> an invalid path ("\\\\") and resulting in an EFI_NOT_FOUND result from the
>>> CurrentHandle->Open() call.
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>>> ---
>>>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25
>>> ++++++++++++++------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> index 5dc893833a46..28e28e5f67d5 100644
>>> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> @@ -816,10 +816,25 @@ FileHandleGetFileName (
>>>          Status = EFI_OUT_OF_RESOURCES;
>>>          break;
>>>        } else {
>>> +        //
>>> +        // Prepare to move to the parent directory.
>>> +        // Also determine whether CurrentHandle refers to the Root
>directory.
>>> +        //
>>> +        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>>> + L"..", EFI_FILE_MODE_READ, 0);
>>>          //
>>>          // We got info... do we have a name? if yes precede the current path
>>> with it...
>>>          //
>>> -        if (StrLen (FileInfo->FileName) == 0) {
>>> +        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
>>> +          //
>>> +          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
>>> +          // CurrentHandle refers to the Root directory.  As this loop ensures
>>> +          // FullFileName is starting with '\\' at all times, signal success
>>> +          // and exit the loop.
>>> +          // While FileInfo->FileName could theoretically be a value other than
>>> +          // '\0' or '\\', '\\' is guaranteed to be supported by the
>>> +          // specification and hence its value can safely be ignored.
>>> +          //
>>> +          Status = EFI_SUCCESS;
>>>            if (*FullFileName == NULL) {
>>>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
>>> NULL));
>>>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0); @@
>-
>>> 837,15 +852,11 @@ FileHandleGetFileName (
>>>            FreePool(FileInfo);
>>>          }
>>>        }
>>> +
>>> +      FileHandleClose(CurrentHandle);
>>>        //
>>>        // Move to the parent directory
>>>        //
>>> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>>> L"..", EFI_FILE_MODE_READ, 0);
>>> -      if (EFI_ERROR (Status)) {
>>> -        break;
>>> -      }
>>> -
>>> -      FileHandleClose(CurrentHandle);
>>>        CurrentHandle = NextHigherHandle;
>>>      }
>>>    } else if (Status == EFI_NOT_FOUND) {
>>> --
>>> 2.23.0.windows.1
>>>
>>>
>>>
>
>
>

Re: [edk2-devel] [PATCH] MdePkg/UefiDebugLibConOut: Pass the correct buffer size

[Liming Gao]

Push @ 5ae6c993ab75c694831547b7436543a41d60458a

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Monday, October 21, 2019 11:12 AM
>To: Marvin.Haeuser@outlook.com; devel@edk2.groups.io
>Cc: vit9696@protonmail.com; Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: Re: [edk2-devel] [PATCH] MdePkg/UefiDebugLibConOut: Pass the
>correct buffer size
>
>Reviewed-by: Liming Gao <liming.gao@intel.com>
>
>>-----Original Message-----
>>From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
>>Sent: Sunday, October 20, 2019 8:09 PM
>>To: devel@edk2.groups.io
>>Cc: vit9696@protonmail.com; Kinney, Michael D
>><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>>Subject: [PATCH] MdePkg/UefiDebugLibConOut: Pass the correct buffer size
>>
>>From: Marvin Haeuser <mhaeuser@outlook.de>
>>
>>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2302
>>
>>The second argument of "UnicodeVSPrintAsciiFormat" is "BufferSize",
>>which takes the size of the buffer in bytes. Replace the currently
>>used MAX_DEBUG_MESSAGE_LENGTH usage, which is the buffer's length,
>>with the actual buffer size.
>>
>>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>Cc: Liming Gao <liming.gao@intel.com>
>>Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>>---
>> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>index cf168d05cf21..8ea38ea7cc7c 100644
>>--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>>@@ -104,9 +104,9 @@ DebugPrintMarker (
>>     // Convert the DEBUG() message to a Unicode String
>>
>>     //
>>
>>     if (BaseListMarker == NULL) {
>>
>>-      UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,
>>Format, VaListMarker);
>>
>>+      UnicodeVSPrintAsciiFormat (Buffer, sizeof (Buffer), Format,
>>VaListMarker);
>>
>>     } else {
>>
>>-      UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,
>>Format, BaseListMarker);
>>
>>+      UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format,
>>BaseListMarker);
>>
>>     }
>>
>>
>>
>>
>>
>>--
>>2.23.0.windows.1
>
>
>

Re: [PATCH] UefiShellCommandLib: Default to first found UC for unsupported PlatformLang

[Ni, Ray]

I am ok to this change.

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Thursday, October 24, 2019 9:25 AM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; devel@edk2.groups.io
> Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>
> Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC for
> unsupported PlatformLang
> 
> Hi Ray,
> 
> This patch would set the default language of shell to the first found language
> instead of ASSERT when the matched language is not found. What do you
> think of this change? I don't know the reason of assert. If it is required to
> ASSERT to show the user the shell language should be matched with the
> platform language. Then the patch is inappropriate. If not, the patch is fine.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> > Sent: Sunday, October 20, 2019 8:09 PM
> > To: devel@edk2.groups.io
> > Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> > <zhichao.gao@intel.com>
> > Subject: [PATCH] UefiShellCommandLib: Default to first found UC for
> > unsupported PlatformLang
> >
> > From: Marvin Haeuser <mhaeuser@outlook.de>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2300
> >
> > On some firmwares PlatformLang is set to the local language (e.g.
> > ru-RU), however there is no Unicode Collation protocol instance that
> supports it.
> > As for missing PlatformLang, fall back to the first found instance.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> > ---
> >  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 6
> +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > index 4c48b65fbc1d..345808a1eac6 100644
> > --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > @@ -107,9 +107,13 @@ CommandInit(
> >         //       // Without clue provided use the first Unicode Collation2
> protocol.+
> > // This may happen when PlatformLang is NULL or when no installed
> > Unicode+      // Collation2 protocol instance supports PlatformLang.       //-
> if
> > (PlatformLang == NULL) {+      if (gUnicodeCollation == NULL)
> > {         gUnicodeCollation = Uc;+      }+      if (PlatformLang == NULL)
> > {         break;       } --
> > 2.23.0.windows.1

Re: [PATCH] UefiShellCommandLib: Default to first found UC for unsupported PlatformLang

[Gao, Zhichao]

Then there is no requirement to force ASSERT when the platform Language is mismatch with the shell language.
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

> -----Original Message-----
> From: Ni, Ray
> Sent: Tuesday, November 5, 2019 11:03 AM
> To: Gao, Zhichao <zhichao.gao@intel.com>; Marvin Häuser
> <Marvin.Haeuser@outlook.com>; devel@edk2.groups.io
> Cc: vit9696@protonmail.com
> Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC for
> unsupported PlatformLang
> 
> I am ok to this change.
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Thursday, October 24, 2019 9:25 AM
> > To: Marvin Häuser <Marvin.Haeuser@outlook.com>;
> devel@edk2.groups.io
> > Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC
> > for unsupported PlatformLang
> >
> > Hi Ray,
> >
> > This patch would set the default language of shell to the first found
> > language instead of ASSERT when the matched language is not found.
> > What do you think of this change? I don't know the reason of assert.
> > If it is required to ASSERT to show the user the shell language should
> > be matched with the platform language. Then the patch is inappropriate. If
> not, the patch is fine.
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> > > Sent: Sunday, October 20, 2019 8:09 PM
> > > To: devel@edk2.groups.io
> > > Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> > > <zhichao.gao@intel.com>
> > > Subject: [PATCH] UefiShellCommandLib: Default to first found UC for
> > > unsupported PlatformLang
> > >
> > > From: Marvin Haeuser <mhaeuser@outlook.de>
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2300
> > >
> > > On some firmwares PlatformLang is set to the local language (e.g.
> > > ru-RU), however there is no Unicode Collation protocol instance that
> > supports it.
> > > As for missing PlatformLang, fall back to the first found instance.
> > >
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> > > ---
> > >  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 6
> > +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > > a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > index 4c48b65fbc1d..345808a1eac6 100644
> > > --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > @@ -107,9 +107,13 @@ CommandInit(
> > >         //       // Without clue provided use the first Unicode Collation2
> > protocol.+
> > > // This may happen when PlatformLang is NULL or when no installed
> > > Unicode+      // Collation2 protocol instance supports PlatformLang.       //-
> > if
> > > (PlatformLang == NULL) {+      if (gUnicodeCollation == NULL)
> > > {         gUnicodeCollation = Uc;+      }+      if (PlatformLang == NULL)
> > > {         break;       } --
> > > 2.23.0.windows.1

Re: [PATCH] UefiShellCommandLib: Default to first found UC for unsupported PlatformLang

[Gao, Zhichao]

Sorry for missing check the commit message title's length. The length should be within 72 chars (less than not equal).
So I advise to change the titile to "ShellPkg/CommandLib: Use first found UC when PlatformLang not support".
With that addressed, then take my R-B.

Thanks,
Zhichao

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Tuesday, November 5, 2019 12:52 PM
> To: Ni, Ray <ray.ni@intel.com>; Marvin Häuser
> <Marvin.Haeuser@outlook.com>; devel@edk2.groups.io
> Cc: vit9696@protonmail.com
> Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC for
> unsupported PlatformLang
> 
> Then there is no requirement to force ASSERT when the platform Language is
> mismatch with the shell language.
> Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Tuesday, November 5, 2019 11:03 AM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; Marvin Häuser
> > <Marvin.Haeuser@outlook.com>; devel@edk2.groups.io
> > Cc: vit9696@protonmail.com
> > Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC
> > for unsupported PlatformLang
> >
> > I am ok to this change.
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > Sent: Thursday, October 24, 2019 9:25 AM
> > > To: Marvin Häuser <Marvin.Haeuser@outlook.com>;
> > devel@edk2.groups.io
> > > Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [PATCH] UefiShellCommandLib: Default to first found UC
> > > for unsupported PlatformLang
> > >
> > > Hi Ray,
> > >
> > > This patch would set the default language of shell to the first
> > > found language instead of ASSERT when the matched language is not
> found.
> > > What do you think of this change? I don't know the reason of assert.
> > > If it is required to ASSERT to show the user the shell language
> > > should be matched with the platform language. Then the patch is
> > > inappropriate. If
> > not, the patch is fine.
> > >
> > > Thanks,
> > > Zhichao
> > >
> > > > -----Original Message-----
> > > > From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> > > > Sent: Sunday, October 20, 2019 8:09 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: vit9696@protonmail.com; Ni, Ray <ray.ni@intel.com>; Gao,
> > > > Zhichao <zhichao.gao@intel.com>
> > > > Subject: [PATCH] UefiShellCommandLib: Default to first found UC
> > > > for unsupported PlatformLang
> > > >
> > > > From: Marvin Haeuser <mhaeuser@outlook.de>
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2300
> > > >
> > > > On some firmwares PlatformLang is set to the local language (e.g.
> > > > ru-RU), however there is no Unicode Collation protocol instance
> > > > that
> > > supports it.
> > > > As for missing PlatformLang, fall back to the first found instance.
> > > >
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > > Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
> > > > ---
> > > >  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 6
> > > +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git
> > > > a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > > b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > > index 4c48b65fbc1d..345808a1eac6 100644
> > > > --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > > +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> > > > @@ -107,9 +107,13 @@ CommandInit(
> > > >         //       // Without clue provided use the first Unicode Collation2
> > > protocol.+
> > > > // This may happen when PlatformLang is NULL or when no installed
> > > > Unicode+      // Collation2 protocol instance supports PlatformLang.
> //-
> > > if
> > > > (PlatformLang == NULL) {+      if (gUnicodeCollation == NULL)
> > > > {         gUnicodeCollation = Uc;+      }+      if (PlatformLang == NULL)
> > > > {         break;       } --
> > > > 2.23.0.windows.1