public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp
@ 2016-08-11  9:54 Dandan Bi
  2016-08-11 10:52 ` Gao, Liming
  0 siblings, 1 reply; 2+ messages in thread
From: Dandan Bi @ 2016-08-11  9:54 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Eric Dong

This patch is to enhance SetupBrowser to handle following two cases:
1. When searching BlockName in AltResp, the hex digits of related BlockName
   in AltResp may be in uppercase.
2. When converting the Value in AltResp to HiiValue, the length of value
   string is bigger than the length of StorageWidth of the question.

Notes:
V2:
- Add some comments to make the logic clear.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
---
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 61 +++++++++++++++-----------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index 66c4313..cd3c8cc 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -1427,44 +1427,48 @@ BufferToValue (
   }
   TempChar = *StringPtr;
   *StringPtr = L'\0';
 
   LengthStr = StrLen (Value);
+
+  //
+  // Value points to a Unicode hexadecimal string, we need to convert the string to the value with CHAR16/UINT8...type.
+  // When generating the Value string, we follow this rule: 1 byte -> 2 Unicode characters (for string: 2 byte(CHAR16) ->4 Unicode characters).
+  // So the maximum value string length of a question is : Question->StorageWidth * 2.
+  // If the value string length > Question->StorageWidth * 2, only set the string length as Question->StorageWidth * 2, then convert.
+  //
+  if (LengthStr > (UINTN) Question->StorageWidth * 2) {
+    Length = (UINTN) Question->StorageWidth * 2;
+  } else {
+    Length = LengthStr;
+  }
+
   Status    = EFI_SUCCESS;
   if (!IsBufferStorage && IsString) {
     //
     // Convert Config String to Unicode String, e.g "0041004200430044" => "ABCD"
     // Add string tail char L'\0' into Length
     //
-    Length    = Question->StorageWidth + sizeof (CHAR16);
-    if (Length < ((LengthStr / 4 + 1) * 2)) {
-      Status = EFI_BUFFER_TOO_SMALL;
-    } else {
-      DstBuf = (CHAR16 *) Dst;
-      ZeroMem (TemStr, sizeof (TemStr));
-      for (Index = 0; Index < LengthStr; Index += 4) {
-        StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
-        DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
-      }
-      //
-      // Add tailing L'\0' character
-      //
-      DstBuf[Index/4] = L'\0';
+    DstBuf = (CHAR16 *) Dst;
+    ZeroMem (TemStr, sizeof (TemStr));
+    for (Index = 0; Index < Length; Index += 4) {
+      StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
+      DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
     }
+    //
+    // Add tailing L'\0' character
+    //
+    DstBuf[Index/4] = L'\0';
   } else {
-    if (Question->StorageWidth < ((LengthStr + 1) / 2)) {
-      Status = EFI_BUFFER_TOO_SMALL;
-    } else {
-      ZeroMem (TemStr, sizeof (TemStr));
-      for (Index = 0; Index < LengthStr; Index ++) {
-        TemStr[0] = Value[LengthStr - Index - 1];
-        DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
-        if ((Index & 1) == 0) {
-          Dst [Index/2] = DigitUint8;
-        } else {
-          Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
-        }
+    ZeroMem (TemStr, sizeof (TemStr));
+    for (Index = 0; Index < Length; Index ++) {
+      TemStr[0] = Value[LengthStr - Index - 1];
+      DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
+      if ((Index & 1) == 0) {
+        Dst [Index/2] = DigitUint8;
+      } else {
+        Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
       }
     }
   }
 
   *StringPtr = TempChar;
@@ -3755,10 +3759,15 @@ GetOffsetFromConfigResp (
   //
   // Type is EFI_HII_VARSTORE_EFI_VARIABLE or EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER
   //
 
   //
+  // Convert all hex digits in ConfigResp to lower case before searching.
+  //
+  HiiToLower (ConfigResp);
+
+  //
   // 1. Directly use Question->BlockName to find.
   //
   RequestElement = StrStr (ConfigResp, Question->BlockName);
   if (RequestElement != NULL) {
     //
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp
  2016-08-11  9:54 [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp Dandan Bi
@ 2016-08-11 10:52 ` Gao, Liming
  0 siblings, 0 replies; 2+ messages in thread
From: Gao, Liming @ 2016-08-11 10:52 UTC (permalink / raw)
  To: Bi, Dandan, edk2-devel@lists.01.org; +Cc: Dong, Eric

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

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Thursday, August 11, 2016 5:54 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH v2] MdeModulePkg/Browser: Enhance the logic
> when getting value from AltResp
> 
> This patch is to enhance SetupBrowser to handle following two cases:
> 1. When searching BlockName in AltResp, the hex digits of related
> BlockName
>    in AltResp may be in uppercase.
> 2. When converting the Value in AltResp to HiiValue, the length of value
>    string is bigger than the length of StorageWidth of the question.
> 
> Notes:
> V2:
> - Add some comments to make the logic clear.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> Reviewed-by: Eric Dong <eric.dong@intel.com>
> ---
>  MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 61
> +++++++++++++++-----------
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> index 66c4313..cd3c8cc 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> @@ -1427,44 +1427,48 @@ BufferToValue (
>    }
>    TempChar = *StringPtr;
>    *StringPtr = L'\0';
> 
>    LengthStr = StrLen (Value);
> +
> +  //
> +  // Value points to a Unicode hexadecimal string, we need to convert the
> string to the value with CHAR16/UINT8...type.
> +  // When generating the Value string, we follow this rule: 1 byte -> 2
> Unicode characters (for string: 2 byte(CHAR16) ->4 Unicode characters).
> +  // So the maximum value string length of a question is : Question-
> >StorageWidth * 2.
> +  // If the value string length > Question->StorageWidth * 2, only set the
> string length as Question->StorageWidth * 2, then convert.
> +  //
> +  if (LengthStr > (UINTN) Question->StorageWidth * 2) {
> +    Length = (UINTN) Question->StorageWidth * 2;
> +  } else {
> +    Length = LengthStr;
> +  }
> +
>    Status    = EFI_SUCCESS;
>    if (!IsBufferStorage && IsString) {
>      //
>      // Convert Config String to Unicode String, e.g "0041004200430044" =>
> "ABCD"
>      // Add string tail char L'\0' into Length
>      //
> -    Length    = Question->StorageWidth + sizeof (CHAR16);
> -    if (Length < ((LengthStr / 4 + 1) * 2)) {
> -      Status = EFI_BUFFER_TOO_SMALL;
> -    } else {
> -      DstBuf = (CHAR16 *) Dst;
> -      ZeroMem (TemStr, sizeof (TemStr));
> -      for (Index = 0; Index < LengthStr; Index += 4) {
> -        StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
> -        DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
> -      }
> -      //
> -      // Add tailing L'\0' character
> -      //
> -      DstBuf[Index/4] = L'\0';
> +    DstBuf = (CHAR16 *) Dst;
> +    ZeroMem (TemStr, sizeof (TemStr));
> +    for (Index = 0; Index < Length; Index += 4) {
> +      StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
> +      DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
>      }
> +    //
> +    // Add tailing L'\0' character
> +    //
> +    DstBuf[Index/4] = L'\0';
>    } else {
> -    if (Question->StorageWidth < ((LengthStr + 1) / 2)) {
> -      Status = EFI_BUFFER_TOO_SMALL;
> -    } else {
> -      ZeroMem (TemStr, sizeof (TemStr));
> -      for (Index = 0; Index < LengthStr; Index ++) {
> -        TemStr[0] = Value[LengthStr - Index - 1];
> -        DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
> -        if ((Index & 1) == 0) {
> -          Dst [Index/2] = DigitUint8;
> -        } else {
> -          Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
> -        }
> +    ZeroMem (TemStr, sizeof (TemStr));
> +    for (Index = 0; Index < Length; Index ++) {
> +      TemStr[0] = Value[LengthStr - Index - 1];
> +      DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
> +      if ((Index & 1) == 0) {
> +        Dst [Index/2] = DigitUint8;
> +      } else {
> +        Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
>        }
>      }
>    }
> 
>    *StringPtr = TempChar;
> @@ -3755,10 +3759,15 @@ GetOffsetFromConfigResp (
>    //
>    // Type is EFI_HII_VARSTORE_EFI_VARIABLE or
> EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER
>    //
> 
>    //
> +  // Convert all hex digits in ConfigResp to lower case before searching.
> +  //
> +  HiiToLower (ConfigResp);
> +
> +  //
>    // 1. Directly use Question->BlockName to find.
>    //
>    RequestElement = StrStr (ConfigResp, Question->BlockName);
>    if (RequestElement != NULL) {
>      //
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-08-11 10:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-11  9:54 [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp Dandan Bi
2016-08-11 10:52 ` Gao, Liming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox