Re: [edk2-devel] [edk2-redfish-client][PATCH v2 2/2] RedfishClientPkg/Features: release resources

2024-04-09 Thread Chang, Abner via groups.io 
[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Monday, April 1, 2024 10:18 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nick Ramirez 
> Subject: [edk2-redfish-client][PATCH v2 2/2] RedfishClientPkg/Features: 
> release
> resources
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> -Release Etag and PendingSettingUri resources.
> -Update function header for GetHttpResponseEtag() and
> GetHttpResponseLocation(). Caller has to release returned
> memory buffer from these two functions.
> -Fix typo.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nick Ramirez 
> ---
>  .../Features/Bios/v1_0_9/Common/BiosCommon.c  |  2 +-
>  .../Features/Bios/v1_0_9/Dxe/BiosDxe.c| 20 --
>  .../BootOption/v1_0_4/Dxe/BootOptionDxe.c | 33 -
>  .../BootOptionCollectionDxe.c |  2 +-
>  .../v1_13_0/Dxe/ComputerSystemDxe.c   | 33 -
>  .../v1_5_0/Dxe/ComputerSystemDxe.c| 33 -
>  .../ComputerSystemCollectionDxe.c |  2 +-
>  .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c| 37 ++-
>  .../MemoryCollectionDxe/MemoryCollectionDxe.c |  2 +-
>  .../RedfishFeatureUtilityLib.c|  2 +
>  10 files changed, 118 insertions(+), 48 deletions(-)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index f40fe215a..5dc97876c 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -815,7 +815,7 @@ HandleResource (
>
>//
>// Check and see if target property exist or not even when collection 
> member
> exists.
> -  // If not, we sill do provision.
> +  // If not, we still do provision.
>//
>DEBUG ((REDFISH_DEBUG_TRACE, "%a Check for %s\n", __func__, Uri));
>Status = EdkIIRedfishResourceConfigCheck (, Uri, NULL);
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index bb64ef862..5955917f2 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -158,8 +158,12 @@ RedfishResourceConsumeResource (
>//
>// Searching for etag in HTTP response header
>//
> -  Etag = NULL;
> -  GetHttpResponseEtag (ExpectedResponse, );
> +  Etag   = NULL;
> +  Status = GetHttpResponseEtag (ExpectedResponse, );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> __func__));
> +  }
> +
>Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
>if (EFI_ERROR (Status)) {
>  DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s: %r\n",
> __func__, Private->Uri, Status));
> @@ -338,8 +342,12 @@ RedfishResourceCheck (
>//
>// Find etag in HTTP response header
>//
> -  Etag = NULL;
> -  GetHttpResponseEtag (, );
> +  Etag   = NULL;
> +  Status = GetHttpResponseEtag (, );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> __func__));
> +  }
> +
>Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
>if (EFI_ERROR (Status)) {
>  DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> __func__, Uri, Status));
> @@ -348,6 +356,10 @@ RedfishResourceCheck (
>//
>// Release resource
>//
> +  if (Etag != NULL) {
> +FreePool (Etag);
> +  }
> +
>RedfishHttpFreeResponse ();
>Private->Payload = NULL;
>
> diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> index 5a66fe59e..1a1262403 100644
> --- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> +++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> @@ -122,12 +122,13 @@ RedfishResourceConsumeResource (
>//
>// Check and see if "@Redfish.Settings" exist or not.
>//
> -  Status = GetPendingSettings (
> - Private->RedfishService,
> - Response.Payload,
> - ,
> - 
> - );
> +  PendingSettingUri = NULL;
> +  Status= GetPendingSettings (
> +Private->RedfishService,
> +Response.Payload,
> +,
> +
> +);
>if (!EFI_ERROR (Status)) {
>  DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: @Redfish.Settings
> found: %s\n", __func__, PendingSettingUri));
>  SetRedfishSettingsObjectsUri (Private->Uri, PendingSettingUri);
> @@ -147,8 +148,12 @@ RedfishResourceConsumeResource (
>//
>// Find etag in

[edk2-devel] [edk2-redfish-client][PATCH v2 2/2] RedfishClientPkg/Features: release resources

2024-04-01 Thread Nickle Wang via groups.io 
-Release Etag and PendingSettingUri resources.
-Update function header for GetHttpResponseEtag() and
GetHttpResponseLocation(). Caller has to release returned
memory buffer from these two functions.
-Fix typo.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 
---
 .../Features/Bios/v1_0_9/Common/BiosCommon.c  |  2 +-
 .../Features/Bios/v1_0_9/Dxe/BiosDxe.c| 20 --
 .../BootOption/v1_0_4/Dxe/BootOptionDxe.c | 33 -
 .../BootOptionCollectionDxe.c |  2 +-
 .../v1_13_0/Dxe/ComputerSystemDxe.c   | 33 -
 .../v1_5_0/Dxe/ComputerSystemDxe.c| 33 -
 .../ComputerSystemCollectionDxe.c |  2 +-
 .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c| 37 ++-
 .../MemoryCollectionDxe/MemoryCollectionDxe.c |  2 +-
 .../RedfishFeatureUtilityLib.c|  2 +
 10 files changed, 118 insertions(+), 48 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c 
b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index f40fe215a..5dc97876c 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -815,7 +815,7 @@ HandleResource (
 
   //
   // Check and see if target property exist or not even when collection member 
exists.
-  // If not, we sill do provision.
+  // If not, we still do provision.
   //
   DEBUG ((REDFISH_DEBUG_TRACE, "%a Check for %s\n", __func__, Uri));
   Status = EdkIIRedfishResourceConfigCheck (, Uri, NULL);
diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c 
b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index bb64ef862..5955917f2 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -158,8 +158,12 @@ RedfishResourceConsumeResource (
   //
   // Searching for etag in HTTP response header
   //
-  Etag = NULL;
-  GetHttpResponseEtag (ExpectedResponse, );
+  Etag   = NULL;
+  Status = GetHttpResponseEtag (ExpectedResponse, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", 
__func__));
+  }
+
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s: %r\n", 
__func__, Private->Uri, Status));
@@ -338,8 +342,12 @@ RedfishResourceCheck (
   //
   // Find etag in HTTP response header
   //
-  Etag = NULL;
-  GetHttpResponseEtag (, );
+  Etag   = NULL;
+  Status = GetHttpResponseEtag (, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", 
__func__));
+  }
+
   Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", 
__func__, Uri, Status));
@@ -348,6 +356,10 @@ RedfishResourceCheck (
   //
   // Release resource
   //
+  if (Etag != NULL) {
+FreePool (Etag);
+  }
+
   RedfishHttpFreeResponse ();
   Private->Payload = NULL;
 
diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c 
b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
index 5a66fe59e..1a1262403 100644
--- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
+++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
@@ -122,12 +122,13 @@ RedfishResourceConsumeResource (
   //
   // Check and see if "@Redfish.Settings" exist or not.
   //
-  Status = GetPendingSettings (
- Private->RedfishService,
- Response.Payload,
- ,
- 
- );
+  PendingSettingUri = NULL;
+  Status= GetPendingSettings (
+Private->RedfishService,
+Response.Payload,
+,
+
+);
   if (!EFI_ERROR (Status)) {
 DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: @Redfish.Settings found: 
%s\n", __func__, PendingSettingUri));
 SetRedfishSettingsObjectsUri (Private->Uri, PendingSettingUri);
@@ -147,8 +148,12 @@ RedfishResourceConsumeResource (
   //
   // Find etag in HTTP response header
   //
-  Etag = NULL;
-  GetHttpResponseEtag (ExpectedResponse, );
+  Etag   = NULL;
+  Status = GetHttpResponseEtag (ExpectedResponse, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", 
__func__));
+  }
+
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s: %r\n", 
__func__, Private->Uri, Status));
@@ -170,6 +175,10 @@ RedfishResourceConsumeResource (
 Private->Json = NULL;
   }
 
+  if (PendingSettingUri != NULL) {
+FreePool (PendingSettingUri);
+  }
+
   return Status;
 }
 
@@ -323,8