1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 00/12] OvmfPkg: Replace the OVMF-specific SataControllerDxe with the generic one
  4. View patch

[edk2-devel] [PATCH v2 01/12] MdeModulePkg/SataControllerDxe: Clean up error handling in Start()

Pedro Falcato posted 12 patches 2 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 01/12] MdeModulePkg/SataControllerDxe: Clean up error handling in Start()
Posted by Pedro Falcato 2 years, 9 months ago 
Clean up error handling using cascading labels + goto.

(port of commit 379b179 + bcab714)

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
 .../Pci/SataControllerDxe/SataController.c    | 80 ++++++++-----------
 1 file changed, 35 insertions(+), 45 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index f661efaec7e9..d67a3e69f649 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -375,8 +375,7 @@ SataControllerStart (
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "SataControllerStart error. return status = %r\n", Status));
-    return Status;
+    goto Bail;
   }
 
   //
@@ -385,7 +384,7 @@ SataControllerStart (
   Private = AllocateZeroPool (sizeof (EFI_SATA_CONTROLLER_PRIVATE_DATA));
   if (Private == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
-    goto Done;
+    goto ClosePciIo;
   }
 
   //
@@ -412,7 +411,7 @@ SataControllerStart (
                     &Private->OriginalPciAttributes
                     );
   if (EFI_ERROR (Status)) {
-    goto Done;
+    goto FreeSataPrivate;
   }
 
   DEBUG ((
@@ -428,7 +427,7 @@ SataControllerStart (
                     &Supports
                     );
   if (EFI_ERROR (Status)) {
-    goto Done;
+    goto FreeSataPrivate;
   }
 
   DEBUG ((DEBUG_INFO, "Supported PCI Attributes = 0x%llx\n", Supports));
@@ -441,7 +440,7 @@ SataControllerStart (
                        NULL
                        );
   if (EFI_ERROR (Status)) {
-    goto Done;
+    goto FreeSataPrivate;
   }
 
   DEBUG ((DEBUG_INFO, "Enabled PCI Attributes = 0x%llx\n", Supports));
@@ -456,7 +455,7 @@ SataControllerStart (
                         );
   if (EFI_ERROR (Status)) {
     ASSERT (FALSE);
-    goto Done;
+    goto RestorePciAttributes;
   }
 
   if (IS_PCI_IDE (&PciData)) {
@@ -470,7 +469,7 @@ SataControllerStart (
     DEBUG ((DEBUG_INFO, "Ports Implemented(PI) = 0x%x\n", Data32));
     if (Data32 == 0) {
       Status = EFI_UNSUPPORTED;
-      goto Done;
+      goto RestorePciAttributes;
     }
 
     MaxPortNumber = 31;
@@ -502,19 +501,19 @@ SataControllerStart (
   Private->DisqualifiedModes = AllocateZeroPool ((sizeof (EFI_ATA_COLLECTIVE_MODE)) * TotalCount);
   if (Private->DisqualifiedModes == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
-    goto Done;
+    goto RestorePciAttributes;
   }
 
   Private->IdentifyData = AllocateZeroPool ((sizeof (EFI_IDENTIFY_DATA)) * TotalCount);
   if (Private->IdentifyData == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
-    goto Done;
+    goto FreeDisqualifiedModes;
   }
 
   Private->IdentifyValid = AllocateZeroPool ((sizeof (BOOLEAN)) * TotalCount);
   if (Private->IdentifyValid == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
-    goto Done;
+    goto FreeIdentifyData;
   }
 
   //
@@ -527,46 +526,37 @@ SataControllerStart (
                   NULL
                   );
 
-Done:
   if (EFI_ERROR (Status)) {
-    gBS->CloseProtocol (
-           Controller,
-           &gEfiPciIoProtocolGuid,
-           This->DriverBindingHandle,
-           Controller
-           );
-    if (Private != NULL) {
-      if (Private->DisqualifiedModes != NULL) {
-        FreePool (Private->DisqualifiedModes);
-      }
-
-      if (Private->IdentifyData != NULL) {
-        FreePool (Private->IdentifyData);
-      }
-
-      if (Private->IdentifyValid != NULL) {
-        FreePool (Private->IdentifyValid);
-      }
-
-      if (Private->PciAttributesChanged) {
-        //
-        // Restore original PCI attributes
-        //
-        PciIo->Attributes (
-                 PciIo,
-                 EfiPciIoAttributeOperationSet,
-                 Private->OriginalPciAttributes,
-                 NULL
-                 );
-      }
-
-      FreePool (Private);
-    }
+    goto FreeIdentifyValid;
   }
 
   DEBUG ((DEBUG_INFO, "SataControllerStart end with %r\n", Status));
 
   return Status;
+
+FreeIdentifyValid:
+  FreePool (Private->IdentifyValid);
+FreeIdentifyData:
+  FreePool (Private->IdentifyData);
+FreeDisqualifiedModes:
+  FreePool (Private->DisqualifiedModes);
+RestorePciAttributes:
+  //
+  // Restore original PCI attributes
+  //
+  Private->PciIo->Attributes (
+                    Private->PciIo,
+                    EfiPciIoAttributeOperationSet,
+                    Private->OriginalPciAttributes,
+                    NULL
+                    );
+FreeSataPrivate:
+  FreePool (Private);
+ClosePciIo:
+  gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid, This->DriverBindingHandle, Controller);
+Bail:
+  DEBUG ((DEBUG_ERROR, "SataControllerStart error return status = %r\n", Status));
+  return Status;
 }
 
 /**
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104406): https://edk2.groups.io/g/devel/message/104406
Mute This Topic: https://groups.io/mt/98787854/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 01/12] MdeModulePkg/SataControllerDxe: Clean up error handling in Start()
Posted by Wu, Hao A 2 years, 9 months ago 
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro
> Falcato
> Sent: Wednesday, May 10, 2023 12:32 AM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek <lersek@redhat.com>; Pedro Falcato
> <pedro.falcato@gmail.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>; Wu, Hao A <hao.a.wu@intel.com>; Ni,
> Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH v2 01/12] MdeModulePkg/SataControllerDxe:
> Clean up error handling in Start()
> 
> Clean up error handling using cascading labels + goto.
> 
> (port of commit 379b179 + bcab714)
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> ---
>  .../Pci/SataControllerDxe/SataController.c    | 80 ++++++++-----------
>  1 file changed, 35 insertions(+), 45 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> index f661efaec7e9..d67a3e69f649 100644
> --- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> @@ -375,8 +375,7 @@ SataControllerStart (
>                    EFI_OPEN_PROTOCOL_BY_DRIVER
>                    );
>    if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "SataControllerStart error. return status = %r\n",
> Status));
> -    return Status;
> +    goto Bail;
>    }
> 
>    //
> @@ -385,7 +384,7 @@ SataControllerStart (
>    Private = AllocateZeroPool (sizeof (EFI_SATA_CONTROLLER_PRIVATE_DATA));
>    if (Private == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> -    goto Done;
> +    goto ClosePciIo;
>    }
> 
>    //
> @@ -412,7 +411,7 @@ SataControllerStart (
>                      &Private->OriginalPciAttributes
>                      );
>    if (EFI_ERROR (Status)) {
> -    goto Done;
> +    goto FreeSataPrivate;
>    }
> 
>    DEBUG ((
> @@ -428,7 +427,7 @@ SataControllerStart (
>                      &Supports
>                      );
>    if (EFI_ERROR (Status)) {
> -    goto Done;
> +    goto FreeSataPrivate;
>    }
> 
>    DEBUG ((DEBUG_INFO, "Supported PCI Attributes = 0x%llx\n", Supports));
> @@ -441,7 +440,7 @@ SataControllerStart (
>                         NULL
>                         );
>    if (EFI_ERROR (Status)) {
> -    goto Done;
> +    goto FreeSataPrivate;
>    }
> 
>    DEBUG ((DEBUG_INFO, "Enabled PCI Attributes = 0x%llx\n", Supports));
> @@ -456,7 +455,7 @@ SataControllerStart (
>                          );
>    if (EFI_ERROR (Status)) {
>      ASSERT (FALSE);
> -    goto Done;
> +    goto RestorePciAttributes;
>    }
> 
>    if (IS_PCI_IDE (&PciData)) {
> @@ -470,7 +469,7 @@ SataControllerStart (
>      DEBUG ((DEBUG_INFO, "Ports Implemented(PI) = 0x%x\n", Data32));
>      if (Data32 == 0) {
>        Status = EFI_UNSUPPORTED;
> -      goto Done;
> +      goto RestorePciAttributes;
>      }
> 
>      MaxPortNumber = 31;
> @@ -502,19 +501,19 @@ SataControllerStart (
>    Private->DisqualifiedModes = AllocateZeroPool ((sizeof
> (EFI_ATA_COLLECTIVE_MODE)) * TotalCount);
>    if (Private->DisqualifiedModes == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> -    goto Done;
> +    goto RestorePciAttributes;
>    }
> 
>    Private->IdentifyData = AllocateZeroPool ((sizeof (EFI_IDENTIFY_DATA)) *
> TotalCount);
>    if (Private->IdentifyData == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> -    goto Done;
> +    goto FreeDisqualifiedModes;
>    }
> 
>    Private->IdentifyValid = AllocateZeroPool ((sizeof (BOOLEAN)) * TotalCount);
>    if (Private->IdentifyValid == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> -    goto Done;
> +    goto FreeIdentifyData;
>    }
> 
>    //
> @@ -527,46 +526,37 @@ SataControllerStart (
>                    NULL
>                    );
> 
> -Done:
>    if (EFI_ERROR (Status)) {
> -    gBS->CloseProtocol (
> -           Controller,
> -           &gEfiPciIoProtocolGuid,
> -           This->DriverBindingHandle,
> -           Controller
> -           );
> -    if (Private != NULL) {
> -      if (Private->DisqualifiedModes != NULL) {
> -        FreePool (Private->DisqualifiedModes);
> -      }
> -
> -      if (Private->IdentifyData != NULL) {
> -        FreePool (Private->IdentifyData);
> -      }
> -
> -      if (Private->IdentifyValid != NULL) {
> -        FreePool (Private->IdentifyValid);
> -      }
> -
> -      if (Private->PciAttributesChanged) {
> -        //
> -        // Restore original PCI attributes
> -        //
> -        PciIo->Attributes (
> -                 PciIo,
> -                 EfiPciIoAttributeOperationSet,
> -                 Private->OriginalPciAttributes,
> -                 NULL
> -                 );
> -      }
> -
> -      FreePool (Private);
> -    }
> +    goto FreeIdentifyValid;
>    }
> 
>    DEBUG ((DEBUG_INFO, "SataControllerStart end with %r\n", Status));
> 
>    return Status;
> +
> +FreeIdentifyValid:
> +  FreePool (Private->IdentifyValid);
> +FreeIdentifyData:
> +  FreePool (Private->IdentifyData);
> +FreeDisqualifiedModes:
> +  FreePool (Private->DisqualifiedModes);
> +RestorePciAttributes:
> +  //
> +  // Restore original PCI attributes
> +  //
> +  Private->PciIo->Attributes (
> +                    Private->PciIo,
> +                    EfiPciIoAttributeOperationSet,
> +                    Private->OriginalPciAttributes,
> +                    NULL
> +                    );
> +FreeSataPrivate:
> +  FreePool (Private);
> +ClosePciIo:
> +  gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid, This-
> >DriverBindingHandle, Controller);
> +Bail:
> +  DEBUG ((DEBUG_ERROR, "SataControllerStart error return status = %r\n",
> Status));
> +  return Status;
>  }
> 
>  /**
> --
> 2.40.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104726): https://edk2.groups.io/g/devel/message/104726
Mute This Topic: https://groups.io/mt/98787854/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 01/12] MdeModulePkg/SataControllerDxe: Clean up error handling in Start()
Posted by Laszlo Ersek 2 years, 9 months ago 
On 5/9/23 18:32, Pedro Falcato wrote:
> Clean up error handling using cascading labels + goto.
> 
> (port of commit 379b179 + bcab714)
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> ---
>  .../Pci/SataControllerDxe/SataController.c    | 80 ++++++++-----------
>  1 file changed, 35 insertions(+), 45 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104517): https://edk2.groups.io/g/devel/message/104517
Mute This Topic: https://groups.io/mt/98787854/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.