public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info
@ 2020-06-18  3:24 Dandan Bi
  2020-06-18  6:27 ` Dong, Eric
  2020-06-18 12:22 ` [edk2-devel] " Laszlo Ersek
  0 siblings, 2 replies; 5+ messages in thread
From: Dandan Bi @ 2020-06-18  3:24 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Eric Dong

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


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

* Re: [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info
  2020-06-18  3:24 [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info Dandan Bi
@ 2020-06-18  6:27 ` Dong, Eric
  2020-06-18  8:39   ` Dandan Bi
  2020-06-18 12:22 ` [edk2-devel] " Laszlo Ersek
  1 sibling, 1 reply; 5+ messages in thread
From: Dong, Eric @ 2020-06-18  6:27 UTC (permalink / raw)
  To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming

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


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

* Re: [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info
  2020-06-18  6:27 ` Dong, Eric
@ 2020-06-18  8:39   ` Dandan Bi
  2020-06-19  0:18     ` Dong, Eric
  0 siblings, 1 reply; 5+ messages in thread
From: Dandan Bi @ 2020-06-18  8:39 UTC (permalink / raw)
  To: Dong, Eric, devel@edk2.groups.io; +Cc: Gao, Liming

Hi Eric,

For example:

For one of, if its current value is 4, but there is no option with value 4, then it will pop up mismatch window and the debug message like below.
[DisplayEngine]: Mismatch Formset    : Formset Guid = 9E0C30BC-3F06-4BA6-8288-09179B855DBE.
[DisplayEngine]: Mismatch Form         : FormId = 4096,  Form title = Front Page.
[DisplayEngine]: Mismatch OneOf      : Named = Select Language.
[DisplayEngine]: Mismatch OneOf      : Current value = 4.
[DisplayEngine]: Option 0                     : Option Name = Standard English. Option Value = 0.
[DisplayEngine]: Option 1                     : Option Name = Standard Franτais. Option Value = 1.
[DisplayEngine]: Option 2                     : Option Name = English. Option Value = 2.
[DisplayEngine]: Option 3                     : Option Name = Franτais. Option Value = 3.

For OrderedList, its value is in the type of array, if one of array value is 4, but there is no option with value 4, then it will pop up mismatch window and the debug message like below.
[DisplayEngine]: Mismatch Formset       : Formset Guid = A04A27F4-DF00-4D42-B552-39511302113D.
[DisplayEngine]: Mismatch Form            : FormId = 1,  Form title = My First Setup Page.
[DisplayEngine]: Mismatch OrderedList: Name = Boot Order.
[DisplayEngine]: Mismatch OrderedList: Value Arrary:
                                                                        Value[0] =4.
                                                                        Value[1] =1.
                                                                        Value[2] =3.
                                                                        Value[3] =0.
                                                                        Value[4] =0.
                                                                        Value[5] =0.
                                                                        Value[6] =0.
                                                                        Value[7] =0.
[DisplayEngine]: Option 0                        : Option Name = ATAPI CD. Option Value = 2.
[DisplayEngine]: Option 1                        : Option Name = IDE HDD. Option Value = 1.
[DisplayEngine]: Option 2                        : Option Name = PXE. Option Value = 3.


Thanks,
Dandan
> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com>
> Sent: Thursday, June 18, 2020 2:28 PM
> To: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: RE: [patch] MdeModulePkg/DisplayEngine: Add Debug message to
> show mismatch menu info
> 
> 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


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

* Re: [edk2-devel] [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info
  2020-06-18  3:24 [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info Dandan Bi
  2020-06-18  6:27 ` Dong, Eric
@ 2020-06-18 12:22 ` Laszlo Ersek
  1 sibling, 0 replies; 5+ messages in thread
From: Laszlo Ersek @ 2020-06-18 12:22 UTC (permalink / raw)
  To: devel, dandan.bi; +Cc: Liming Gao, Eric Dong

On 06/18/20 05:24, Dandan Bi wrote:
> 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));

I suggest replacing the open-coded "[DisplayEngine]" strings in this
patch with "[%a]", and passing gEfiCallerBaseName.

This will make no difference in practice (given "BASE_NAME =
DisplayEngine" in "DisplayEngineDxe.inf"), but it's a better pattern --
the resultant debug strings are less verbose, and they'd update
themselves if we ever changed BASE_NAME.

No other comments from me.

Thanks,
Laszlo

> +  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);
> 


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

* Re: [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info
  2020-06-18  8:39   ` Dandan Bi
@ 2020-06-19  0:18     ` Dong, Eric
  0 siblings, 0 replies; 5+ messages in thread
From: Dong, Eric @ 2020-06-19  0:18 UTC (permalink / raw)
  To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming

[-- Attachment #1: Type: text/plain, Size: 11012 bytes --]

Hi Dandan,

Thanks for your detail explanation, add my comments below.

Thanks,
Eric
From: Bi, Dandan <dandan.bi@intel.com>
Sent: Thursday, June 18, 2020 4:39 PM
To: Dong, Eric <eric.dong@intel.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info

Hi Eric,

For example:

For one of, if its current value is 4, but there is no option with value 4, then it will pop up mismatch window and the debug message like below.
[DisplayEngine]: Mismatch Error        : Current value not exists in the option list.                             <= it's better to add this line to show an overview of error info.
[DisplayEngine]: Mismatch Formset    : Formset Guid = 9E0C30BC-3F06-4BA6-8288-09179B855DBE.   Formset title = "####"            <= Title also needs if save happen in system level.
[DisplayEngine]: Mismatch Form         : FormId = 4096,  Form title = Front Page.
[DisplayEngine]: Mismatch OneOf      : Named = Select Language.
[DisplayEngine]: Mismatch OneOf      : Current value = 4.
[DisplayEngine]: Option 0                     : Option Name = Standard English. Option Value = 0.
[DisplayEngine]: Option 1                     : Option Name = Standard Franτais. Option Value = 1.
[DisplayEngine]: Option 2                     : Option Name = English. Option Value = 2.
[DisplayEngine]: Option 3                     : Option Name = Franτais. Option Value = 3.

For OrderedList, its value is in the type of array, if one of array value is 4, but there is no option with value 4, then it will pop up mismatch window and the debug message like below.
[DisplayEngine]: Mismatch Error            : Current one value not exists in the option list.                             <= same as above reason, you can provide better words if you have.
[DisplayEngine]: Mismatch Formset       : Formset Guid = A04A27F4-DF00-4D42-B552-39511302113D.     Formset title = "####"            <= Title also needs if save happen in system level.
[DisplayEngine]: Mismatch Form            : FormId = 1,  Form title = My First Setup Page.
[DisplayEngine]: Mismatch OrderedList: Name = Boot Order.
[DisplayEngine]: Mismatch OrderedList: Value Arrary:
                                                                        Value[0] =4.
                                                                        Value[1] =1.
                                                                        Value[2] =3.
                                                                        Value[3] =0.
                                                                        Value[4] =0.
                                                                        Value[5] =0.
                                                                        Value[6] =0.
                                                                        Value[7] =0.
[DisplayEngine]: Option 0                        : Option Name = ATAPI CD. Option Value = 2.
[DisplayEngine]: Option 1                        : Option Name = IDE HDD. Option Value = 1.
[DisplayEngine]: Option 2                        : Option Name = PXE. Option Value = 3.


Thanks,
Dandan
> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> Sent: Thursday, June 18, 2020 2:28 PM
> To: Bi, Dandan <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Subject: RE: [patch] MdeModulePkg/DisplayEngine: Add Debug message to
> show mismatch menu info
>
> 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<mailto:dandan.bi@intel.com>>
> > Sent: Thursday, June 18, 2020 11:24 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > Cc: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Dong, Eric
> > <eric.dong@intel.com<mailto: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<mailto:liming.gao@intel.com>>
> > Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> > Signed-off-by: Dandan Bi <dandan.bi@intel.com<mailto: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

[-- Attachment #2: Type: text/html, Size: 39225 bytes --]

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

end of thread, other threads:[~2020-06-19  0:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-18  3:24 [patch] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info Dandan Bi
2020-06-18  6:27 ` Dong, Eric
2020-06-18  8:39   ` Dandan Bi
2020-06-19  0:18     ` Dong, Eric
2020-06-18 12:22 ` [edk2-devel] " Laszlo Ersek

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