Re: [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp

["Gao, Liming" <liming.gao@intel.com>]

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