K8:

1) Replace immediate constant with MACRO.
2) Add in ASSERT for NULL before dereferencing pointers and after memory allocation.
3) Add a return statement in ProcessOptions to handle the mismatch between the value stored and the opcode for one-of-option in orderedlist opcode. This case occurs when the one-of-optoin entry in OrderedList get deleted.


git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@7320 6f19259b-4bc3-4df7-8a09-765794883524

MdeModulePkg/Universal/SetupBrowserDxe/InputHandler.c

@@ -235,9 +235,9 @@ GetNumericInput (
   EFI_STATUS              Status;
   UINTN                   Column;
   UINTN                   Row;
-  CHAR16                  InputText[23];
-  CHAR16                  FormattedNumber[22];
-  UINT64                  PreviousNumber[20];
+  CHAR16                  InputText[MAX_NUMERIC_INPUT_WIDTH];
+  CHAR16                  FormattedNumber[MAX_NUMERIC_INPUT_WIDTH - 1];
+  UINT64                  PreviousNumber[MAX_NUMERIC_INPUT_WIDTH - 3];
   UINTN                   Count;
   UINTN                   Loop;
   BOOLEAN                 ManualInput;
@@ -387,6 +387,7 @@ GetNumericInput (
 
     InputText[0] = LEFT_NUMERIC_DELIMITER;
     SetUnicodeMem (InputText + 1, InputWidth, L' ');
+    ASSERT (InputWidth < MAX_NUMERIC_INPUT_WIDTH); 
     InputText[InputWidth + 1] = RIGHT_NUMERIC_DELIMITER;
     InputText[InputWidth + 2] = L'\0';
 
@@ -640,6 +641,7 @@ EnterCarriageReturn:
 
         if (EditValue > Maximum) {
           UpdateStatusBar (INPUT_ERROR, Question->QuestionFlags, TRUE);
+          ASSERT (Count < sizeof (PreviousNumber) / sizeof (PreviousNumber[0]));
           EditValue = PreviousNumber[Count];
           break;
         } else {
@@ -970,6 +972,7 @@ TheKey:
         if (HighlightOptionIndex > 0) {
           HighlightOptionIndex--;
 
+          ASSERT (CurrentOption != NULL);
           SwapListEntries (CurrentOption->Link.BackLink, &CurrentOption->Link);
         }
       }
@@ -997,6 +1000,7 @@ TheKey:
         if (HighlightOptionIndex < (PopUpMenuLines - 1)) {
           HighlightOptionIndex++;
 
+          ASSERT (CurrentOption != NULL);
           SwapListEntries (&CurrentOption->Link, CurrentOption->Link.ForwardLink);
         }
       }
@@ -1096,6 +1100,7 @@ TheKey:
           Link = GetNextNode (&Question->OptionListHead, Link);
         }
       } else {
+        ASSERT (CurrentOption != NULL);
         CopyMem (&Question->HiiValue, &CurrentOption->Value, sizeof (EFI_HII_VALUE));
       }
 

MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c

@@ -188,6 +188,8 @@ DisplayPageFrame (
   CHAR16                 *StrFrontPageBanner;
   UINTN                  Row;
   EFI_SCREEN_DESCRIPTOR  LocalScreen;
+  UINTN                  RowIdx;
+  UINTN                  ColumnIdx;
 
   ZeroMem (&LocalScreen, sizeof (EFI_SCREEN_DESCRIPTOR));
   gST->ConOut->QueryMode (gST->ConOut, gST->ConOut->Mode->Mode, &LocalScreen.RightColumn, &LocalScreen.BottomRow);
@@ -229,9 +231,15 @@ DisplayPageFrame (
            Alignment < BANNER_COLUMNS + (UINT8) LocalScreen.LeftColumn;
            Alignment++
           ) {
-        if (BannerData->Banner[Line - (UINT8) LocalScreen.TopRow][Alignment - (UINT8) LocalScreen.LeftColumn] != 0x0000) {
+        RowIdx = Line - (UINT8) LocalScreen.TopRow;
+        ColumnIdx = Alignment - (UINT8) LocalScreen.LeftColumn;
+
+        ASSERT (RowIdx < BANNER_HEIGHT);
+        ASSERT (ColumnIdx < BANNER_COLUMNS);
+        
+        if (BannerData->Banner[RowIdx][ColumnIdx] != 0x0000) {
           StrFrontPageBanner = GetToken (
-                                BannerData->Banner[Line - (UINT8) LocalScreen.TopRow][Alignment - (UINT8) LocalScreen.LeftColumn],
+                                BannerData->Banner[RowIdx][ColumnIdx],
                                 FrontPageHandle
                                 );
         } else {

MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c

@@ -452,6 +452,7 @@ ProcessOptions (
 
         FreePool (*OptionString);
         *OptionString = NULL;
+        return EFI_NOT_FOUND;
       }
 
       if ((OneOfOption->SuppressExpression != NULL) &&
@@ -722,6 +723,8 @@ ProcessOptions (
           }
 
           TempString = AllocateCopyPool ((Maximum + 1) * sizeof (CHAR16), Question->BufferValue);
+          ASSERT (TempString != NULL);
+          
           TempString[Maximum] = L'\0';
 
           if (StrCmp (StringPtr, TempString) != 0) {
@@ -877,6 +880,7 @@ ProcessHelpString (
   //
   AllocateSize  = 0x20;
   IndexArray    = AllocatePool (AllocateSize * sizeof (UINTN) * 3);
+  ASSERT (IndexArray != NULL);
 
   if (*FormattedString != NULL) {
     FreePool (*FormattedString);
@@ -1012,6 +1016,7 @@ ProcessHelpString (
 
   VirtualLineCount = RowCount * (LineCount / RowCount + (LineCount % RowCount > 0));
   *FormattedString = AllocateZeroPool (VirtualLineCount * (BlockWidth + 1) * sizeof (CHAR16) * 2);
+  ASSERT (*FormattedString != NULL);
 
   for (CurrIndex = 0; CurrIndex < LineCount; CurrIndex ++) {
     *(*FormattedString + CurrIndex * 2 * (BlockWidth + 1)) = (CHAR16) ((IndexArray[CurrIndex*3+2] == 2) ? WIDE_CHAR : NARROW_CHAR);

MdeModulePkg/Universal/SetupBrowserDxe/Ui.h

@@ -26,6 +26,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #define SUBTITLE_INDENT  2
 
+
+//
+// It take 23 characters including the NULL to print a 64 bits number with "[" and "]".
+// pow(2, 64) = [18446744073709551616]
+//
+#define MAX_NUMERIC_INPUT_WIDTH 23
+
 typedef enum {
   UiNoOperation,
   UiDefault,