[PATCH 04/19] ArmPlatformPkg: LcdGraphicsOurputDxe: Add debug asserts

[evan.lloyd@arm.com]

From: Girish Pathak <girish.pathak@arm.com>

This change adds some debug assertions e.g to catch NULL pointer errors
missing in PL11Lcd and HdLcd modules.

This change also improves related error handling code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c       | 44 ++++++++++++++++++--
 ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 43 ++++++++++++++++++-
 ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c                                |  8 ++--
 ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c                             |  8 ++--
 4 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index b9859a56988f7e5be0adbaa49048a683fe586bfe..58dd9f0c77e1bc9af559a71d0c7cce72d71c6da5 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -140,6 +140,7 @@ LcdPlatformInitializeDisplay (
   *                                 buffer in bytes
   *
   * @retval EFI_SUCCESS             Frame buffer memory allocation success.
+  * @retval EFI_INVALID_PARAMETER   VramBaseAddress or VramSize are NULL.
   * @retval !(EFI_SUCCESS)          Other errors.
 **/
 EFI_STATUS
@@ -151,6 +152,13 @@ LcdPlatformGetVram (
   EFI_STATUS              Status;
   EFI_ALLOCATE_TYPE       AllocationType;
 
+  // Check VramBaseAddress and VramSize are not NULL.
+  if (VramBaseAddress == NULL || VramSize == NULL) {
+    ASSERT (VramBaseAddress != NULL);
+    ASSERT (VramSize != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
   // Set the vram size
   *VramSize = LCD_VRAM_SIZE;
 
@@ -169,6 +177,7 @@ LcdPlatformGetVram (
                   VramBaseAddress
                   );
   if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
     return Status;
   }
 
@@ -179,8 +188,8 @@ LcdPlatformGetVram (
                   *VramSize,
                   EFI_MEMORY_WC
                   );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
     gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
     return Status;
   }
@@ -215,6 +224,7 @@ LcdPlatformSetMode (
   EFI_STATUS            Status;
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
     return EFI_INVALID_PARAMETER;
   }
 
@@ -264,6 +274,7 @@ LcdPlatformSetMode (
   *
   * @retval EFI_SUCCESS             Success if the requested mode is found.
   * @retval EFI_INVALID_PARAMETER   Requested mode not found.
+  * @retval EFI_INVALID_PARAMETER   Info is NULL.
 **/
 EFI_STATUS
 LcdPlatformQueryMode (
@@ -271,7 +282,9 @@ LcdPlatformQueryMode (
   OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST  Info
   )
 {
-  if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+  if (ModeNumber >= LcdPlatformGetMaxMode () || Info == NULL) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    ASSERT (Info != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -334,6 +347,28 @@ LcdPlatformGetTimings (
   )
 {
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (HRes == NULL
+    || HSync == NULL
+    || HBackPorch == NULL
+    || HFrontPorch == NULL
+    || VRes == NULL
+    || VSync == NULL
+    || VBackPorch == NULL
+    || VFrontPorch == NULL)
+  {
+    // One of the pointers is NULL
+    ASSERT (HRes != NULL);
+    ASSERT (HSync != NULL);
+    ASSERT (HBackPorch != NULL);
+    ASSERT (HFrontPorch != NULL);
+    ASSERT (VRes != NULL);
+    ASSERT (VSync != NULL);
+    ASSERT (VBackPorch != NULL);
+    ASSERT (VFrontPorch != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -356,6 +391,7 @@ LcdPlatformGetTimings (
   *
   * @retval EFI_SUCCESS             The requested mode is found.
   * @retval EFI_INVALID_PARAMETER   Requested mode not found.
+  * @retval EFI_INVALID_PARAMETER   Bpp is NULL.
 **/
 EFI_STATUS
 LcdPlatformGetBpp (
@@ -363,7 +399,9 @@ LcdPlatformGetBpp (
   OUT LCD_BPP * CONST                    Bpp
   )
 {
-  if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+  if (ModeNumber >= LcdPlatformGetMaxMode () || Bpp == NULL) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    ASSERT (Bpp != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 6ae13f06d8b396ea1c67f0bcd735a9d70f476400..5a4abd4c6f9e84d3d690af7233c1cebfe1ad339b 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -191,6 +191,7 @@ LcdPlatformInitializeDisplay (
   *                                 buffer in bytes
   *
   * @retval EFI_SUCCESS             Frame buffer memory allocation success.
+  * @retval EFI_INVALID_PARAMETER   VramBaseAddress or VramSize is NULL.
   * @retval !(EFI_SUCCESS)          Other errors.
 **/
 EFI_STATUS
@@ -203,6 +204,13 @@ LcdPlatformGetVram (
 
   Status = EFI_SUCCESS;
 
+  // Check VramBaseAddress and VramSize are not NULL.
+  if (VramBaseAddress == NULL || VramSize == NULL) {
+    ASSERT (VramBaseAddress != NULL);
+    ASSERT (VramSize != NULL);
+    return EFI_INVALID_PARAMETER;
+  }
+
   // Is it on the motherboard or on the daughterboard?
   switch (PL111_CLCD_SITE) {
 
@@ -223,6 +231,7 @@ LcdPlatformGetVram (
                     VramBaseAddress
                     );
     if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
       return Status;
     }
 
@@ -233,8 +242,8 @@ LcdPlatformGetVram (
                     *VramSize,
                     EFI_MEMORY_WC
                     );
-    ASSERT_EFI_ERROR (Status);
     if (EFI_ERROR (Status)) {
+      ASSERT_EFI_ERROR (Status);
       gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));
       return Status;
     }
@@ -293,6 +302,7 @@ LcdPlatformSetMode (
   UINT32                SysId;
 
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
     return EFI_INVALID_PARAMETER;
   }
 
@@ -359,6 +369,7 @@ LcdPlatformSetMode (
   *                                 (on success).
   *
   * @retval EFI_SUCCESS             Success if the requested mode is found.
+  * @retval EFI_INVALID_PARAMETER   Info is NULL.
   * @retval EFI_INVALID_PARAMETER   Requested mode not found.
 **/
 EFI_STATUS
@@ -367,7 +378,9 @@ LcdPlatformQueryMode (
   OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info
   )
 {
-  if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+  if (ModeNumber >= LcdPlatformGetMaxMode () || Info == NULL) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    ASSERT (Info != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -415,6 +428,7 @@ LcdPlatformQueryMode (
   *
   * @retval EFI_SUCCESS             Success if the requested mode is found.
   * @retval EFI_INVALID_PARAMETER   Requested mode not found.
+  * @retval EFI_INVALID_PARAMETER   One of the OUT parameters is NULL.
 **/
 EFI_STATUS
 LcdPlatformGetTimings (
@@ -430,6 +444,28 @@ LcdPlatformGetTimings (
   )
 {
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (HRes == NULL
+    || HSync == NULL
+    || HBackPorch == NULL
+    || HFrontPorch == NULL
+    || VRes == NULL
+    || VSync == NULL
+    || VBackPorch == NULL
+    || VFrontPorch == NULL)
+  {
+    // One of the pointers is NULL
+    ASSERT (HRes != NULL);
+    ASSERT (HSync != NULL);
+    ASSERT (HBackPorch != NULL);
+    ASSERT (HFrontPorch != NULL);
+    ASSERT (VRes != NULL);
+    ASSERT (VSync != NULL);
+    ASSERT (VBackPorch != NULL);
+    ASSERT (VFrontPorch != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
@@ -452,6 +488,7 @@ LcdPlatformGetTimings (
   *
   * @retval EFI_SUCCESS             The requested mode is found.
   * @retval EFI_INVALID_PARAMETER   Requested mode not found.
+  * @retval EFI_INVALID_PARAMETER   Bpp is NULL.
 **/
 EFI_STATUS
 LcdPlatformGetBpp (
@@ -460,6 +497,8 @@ LcdPlatformGetBpp (
   )
 {
   if (ModeNumber >= LcdPlatformGetMaxMode ()) {
+    ASSERT (ModeNumber < LcdPlatformGetMaxMode ());
+    ASSERT (Bpp != NULL);
     return EFI_INVALID_PARAMETER;
   }
 
diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
index 5f950579720fb69e0a481f697a5cc4038158b409..a266671a26f01d31e8ddb0cf7cbfe59d2f4dc49c 100644
--- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
+++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c
@@ -109,15 +109,15 @@ LcdSetMode (
              &VBackPorch,
              &VFrontPorch
              );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
   }
 
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
   }
 
   BytesPerPixel = GetBytesPerPixel (LcdBpp);
diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
index 386e6140a69b045f77ee7fa60c4587d8bf4e7d54..f432c8d802614e8a0e4ddab3898f6e0dbf9a1572 100644
--- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
+++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c
@@ -110,15 +110,15 @@ LcdSetMode (
              &VBackPorch,
              &VFrontPorch
              );
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
   }
 
   Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp);
-  ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
+    ASSERT_EFI_ERROR (Status);
+    return Status;
   }
 
   // Disable the CLCD_LcdEn bit
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")