Re: [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info

["Dong, Eric" <eric.dong@intel.com>]

Hi Dandan,

It's good to show more info about the mismatch error. Current end user doesn't know which question need to check if this error appears.

Can you post an example about the error message for one of and for ordered list? We need to make sure the message is easy to be understood and the end user knows what need to do to fix this error.

Thanks,
Eric

> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Thursday, June 18, 2020 11:24 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [patch] MdeModulePkg/DisplayEngine: Add Debug message to
> show mismatch menu info
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2326
> 
> Currently when meet mismatch case for one-of and ordered-list menu, just
> show a popup window to indicate mismatch, no more info for debugging.
> This patch is to add more debug message about mismatch menu info which is
> helpful to debug.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../DisplayEngineDxe/ProcessOptions.c         | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> index e7306f6d04..4331b2903c 100644
> --- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> +++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> @@ -911,10 +911,73 @@ PasswordProcess (
>    FreePool (StringPtr);
> 
>    return Status;
>  }
> 
> +/**
> +  Print some debug message about mismatched menu info.
> +
> +  @param  MenuOption             The MenuOption for this Question.
> +
> +**/
> +VOID
> +PrintMismatchMenuInfo (
> +  IN  UI_MENU_OPTION              *MenuOption
> +)
> +{
> +  CHAR16                          *FormTitleStr;
> +  CHAR16                          *OneOfOptionStr;
> +  CHAR16                          *QuestionName;
> +  LIST_ENTRY                      *Link;
> +  FORM_DISPLAY_ENGINE_STATEMENT   *Question;
> +  EFI_IFR_ORDERED_LIST            *OrderList;
> +  UINTN                           Index;
> +  EFI_HII_VALUE                   HiiValue;
> +  EFI_HII_VALUE                   *QuestionValue;
> +  DISPLAY_QUESTION_OPTION         *Option;
> +  UINT8                           *ValueArray;
> +  UINT8                           ValueType;
> +
> +  Question = MenuOption->ThisTag;
> +  FormTitleStr = GetToken (gFormData->FormTitle, gFormData->HiiHandle);
> +
> +  DEBUG ((DEBUG_ERROR, "\n[DisplayEngine]: Mismatch Formset    :
> Formset Guid = %g.\n", &gFormData->FormSetGuid));
> +  DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Mismatch Form       : FormId
> = %d,  Form title = %s.\n", gFormData->FormId, FormTitleStr));
> +
> +  if (Question->OpCode->OpCode == EFI_IFR_ORDERED_LIST_OP) {
> +    QuestionName = GetToken (((EFI_IFR_ORDERED_LIST*)MenuOption-
> >ThisTag->OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
> +    Link = GetFirstNode (&Question->OptionListHead);
> +    Option = DISPLAY_QUESTION_OPTION_FROM_LINK (Link);
> +    ValueType = Option->OptionOpCode->Type;
> +
> +    DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Mismatch OrderedList: Name
> = %s.\n", QuestionName));
> +    DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Mismatch OrderedList: Value
> + Arrary:\n"));
> +
> +    OrderList = (EFI_IFR_ORDERED_LIST *) Question->OpCode;
> +    for (Index = 0; Index < OrderList->MaxContainers; Index++) {
> +      ValueArray = Question->CurrentValue.Buffer;
> +      HiiValue.Value.u64 = GetArrayData (ValueArray, ValueType, Index);
> +      DEBUG ((DEBUG_ERROR, "                                       Value[%d] =%d.\n",Index,
> HiiValue.Value.u64));
> +    }
> +  } else if (Question->OpCode->OpCode == EFI_IFR_ONE_OF_OP) {
> +    QuestionName = GetToken (((EFI_IFR_ONE_OF*)MenuOption->ThisTag-
> >OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
> +    QuestionValue = &Question->CurrentValue;;
> +    DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Mismatch OneOf      : Named
> = %s.\n", QuestionName));
> +    DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Mismatch OneOf      : Current
> value = %d.\n",QuestionValue->Value.u8));
> +  }
> +
> +  Index = 0;
> +  Link = GetFirstNode (&Question->OptionListHead);
> +  while (!IsNull (&Question->OptionListHead, Link)) {
> +    Option = DISPLAY_QUESTION_OPTION_FROM_LINK (Link);
> +    OneOfOptionStr = GetToken (Option->OptionOpCode->Option,
> gFormData->HiiHandle);
> +    DEBUG ((DEBUG_ERROR, "[DisplayEngine]: Option %d            : Option
> Name = %s. Option Value = %d.\n",Index,OneOfOptionStr,Option-
> >OptionOpCode->Value.u8));
> +    Link = GetNextNode (&Question->OptionListHead, Link);
> +    Index++;
> +  }
> +}
> +
>  /**
>    Process a Question's Option (whether selected or un-selected).
> 
>    @param  MenuOption             The MenuOption for this Question.
>    @param  Selected               TRUE: if Question is selected.
> @@ -1010,10 +1073,15 @@ ProcessOptions (
>            break;
>          }
> 
>          OneOfOption = ValueToOption (Question, &HiiValue);
>          if (OneOfOption == NULL) {
> +          //
> +          // Print debug msg for the mistach menu.
> +          //
> +          PrintMismatchMenuInfo (MenuOption);
> +
>            if (SkipErrorValue) {
>              //
>              // Just try to get the option string, skip the value which not has option.
>              //
>              continue;
> @@ -1102,10 +1170,15 @@ ProcessOptions (
>            continue;
>          }
> 
>          if (!ValueInvalid) {
>            ValueInvalid = TRUE;
> +          //
> +          // Print debug msg for the mistach menu.
> +          //
> +          PrintMismatchMenuInfo (MenuOption);
> +
>            //
>            // Show error message
>            //
>            do {
>              CreateDialog (&Key, gEmptyString, gOptionMismatch, gPressEnter,
> gEmptyString, NULL); @@ -1152,10 +1225,15 @@ ProcessOptions (
>        *OptionString = AllocateZeroPool (BufferSize);
>        ASSERT (*OptionString);
> 
>        OneOfOption = ValueToOption (Question, QuestionValue);
>        if (OneOfOption == NULL) {
> +        //
> +        // Print debug msg for the mistach menu.
> +        //
> +        PrintMismatchMenuInfo (MenuOption);
> +
>          if (SkipErrorValue) {
>            //
>            // Not report error, just get the correct option string info.
>            //
>            Link = GetFirstNode (&Question->OptionListHead);
> --
> 2.18.0.windows.1