[edk2-devel] [edk2-redfish-client][PATCH 0/4] RedfishClientPkg: fix memory leaks and refine code

[edk2-devel] [edk2-redfish-client][PATCH 0/4] RedfishClientPkg: fix memory leaks and refine code

[Mike Maslenkin]

This set contains a trivial fix for a leak reviewed on Feb 1 [1]
and a fix for а leak discussed in [2].

PR: https://github.com/tianocore/edk2-redfish-client/pull/76

[1] https://edk2.groups.io/g/devel/message/114925
[2] https://edk2.groups.io/g/devel/message/114765


Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115684): https://edk2.groups.io/g/devel/message/115684
Mute This Topic: https://groups.io/mt/104479570/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/4] RedfishClientPkg/RedfishFeatureUtilityLib: fix memory leak on error path

[Mike Maslenkin]

Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
---
 .../Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c  | 1 +
 1 file changed, 1 insertion(+)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index e1494471038c..21ce8ddad9d5 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -4010,6 +4010,7 @@ RedfishRemoveUnchangeableProperties (
              (RedfishCS_uint32)AsciiStrSize (*JsonString)
              );
   if (Status != RedfishCS_status_success) {
+    FreePool (UpdatedJsonString);
     return EFI_DEVICE_ERROR;
   }
 
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115685): https://edk2.groups.io/g/devel/message/115685
Mute This Topic: https://groups.io/mt/104479571/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

[Mike Maslenkin]

Use local variable for CollectionUri passed to HandleResource() to avoid
problems in case of Private->Uri is overriden down the call stack.

Suggested-by: Nickle Wang <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index f40f2d85af80..396ec22969b5 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -670,6 +670,7 @@ RedfishExternalResourceResourceFeatureCallback (
   REDFISH_SERVICE                  RedfishService;
   REDFISH_RESOURCE_COMMON_PRIVATE  *Private;
   EFI_STRING                       ResourceUri;
+  EFI_STRING                       CollectionUri;
 
   if (FeatureAction != CallbackActionStartOperation) {
     return EFI_UNSUPPORTED;
@@ -707,19 +708,19 @@ RedfishExternalResourceResourceFeatureCallback (
   //
   // Initialize collection path
   //
-  Private->Uri = RedfishGetUri (ResourceUri);
-  if (Private->Uri == NULL) {
+  CollectionUri = RedfishGetUri (ResourceUri);
+  if (CollectionUri == NULL) {
     ASSERT (FALSE);
     FreePool (ResourceUri);
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Status = HandleResource (Private, Private->Uri);
+  Status = HandleResource (Private, CollectionUri);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n", __func__, Private->Uri, Status));
+    DEBUG ((DEBUG_ERROR, "%a, process external resource: %s failed: %r\n", __func__, CollectionUri, Status));
   }
 
-  FreePool (Private->Uri);
+  FreePool (CollectionUri);
   FreePool (ResourceUri);
   return Status;
 }
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115686): https://edk2.groups.io/g/devel/message/115686
Mute This Topic: https://groups.io/mt/104479572/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/4] RedfishClientPkg/Bios: fix leak of GetPendingSettings URI.

[Mike Maslenkin]

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index 396ec22969b5..8548425c9c83 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -132,6 +132,7 @@ RedfishResourceConsumeResource (
   // Check and see if "@Redfish.Settings" exist or not.
   //
   ZeroMem (&PendingSettingResponse, sizeof (REDFISH_RESPONSE));
+  PendingSettingUri = NULL;
   Status = GetPendingSettings (
              Private->RedfishService,
              Response.Payload,
@@ -206,6 +207,10 @@ RedfishResourceConsumeResource (
     FreePool (Etag);
   }
 
+  if (PendingSettingUri != NULL) {
+    FreePool (PendingSettingUri);
+  }
+
   return Status;
 }
 
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115687): https://edk2.groups.io/g/devel/message/115687
Mute This Topic: https://groups.io/mt/104479573/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/4] RedfishClientPkg: use Json value from a function argument

[Mike Maslenkin]

This patch replaces value Private->Json with Json used as second argument
for RedfishIdentifyResource(). Currently Json argument is not used at all
and the pattern for caller side is:
  Status = RedfishIdentifyResourceCommon (Private, Private->Json);

So in scope of RedfishIdentifyResourceCommon Json actually is the same
value as Private->Json. Let's make code a bit cleaner.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c       | 2 +-
 .../Features/BootOption/v1_0_4/Common/BootOptionCommon.c        | 2 +-
 .../ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c        | 2 +-
 .../ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c         | 2 +-
 RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c   | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index 0ae841499692..f3f993c8782e 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -729,7 +729,7 @@ RedfishIdentifyResourceCommon (
   EFI_STRING                                   EndOfChar;
   REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
 
-  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
+  Supported = RedfishIdentifyResource (Private->Uri, Json);
   if (Supported) {
     Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION, REDPATH_ARRAY_PATTERN, &ConfigLangList);
     if (EFI_ERROR (Status)) {
diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
index 0b9f2bf28434..f471c01c3790 100644
--- a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
+++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
@@ -791,7 +791,7 @@ RedfishIdentifyResourceCommon (
 {
   BOOLEAN  Supported;
 
-  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
+  Supported = RedfishIdentifyResource (Private->Uri, Json);
   if (Supported) {
     return EFI_SUCCESS;
   }
diff --git a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c
index cee6c8bf9ba1..d69fc176ad94 100644
--- a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c
+++ b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c
@@ -848,7 +848,7 @@ RedfishIdentifyResourceCommon (
   EFI_STRING                                   EndOfChar;
   REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
 
-  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
+  Supported = RedfishIdentifyResource (Private->Uri, Json);
   if (Supported) {
     Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION, REDPATH_ARRAY_PATTERN, &ConfigLangList);
     if (EFI_ERROR (Status)) {
diff --git a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
index a67ef3dac283..11bcb5f76cab 100644
--- a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
+++ b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
@@ -1718,7 +1718,7 @@ RedfishIdentifyResourceCommon (
   EFI_STRING                                   EndOfChar;
   REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
 
-  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
+  Supported = RedfishIdentifyResource (Private->Uri, Json);
   if (Supported) {
     Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION, REDPATH_ARRAY_PATTERN, &ConfigLangList);
     if (EFI_ERROR (Status)) {
diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
index eb52c68c5dcb..00a69f748c3c 100644
--- a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
+++ b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
@@ -2516,7 +2516,7 @@ RedfishIdentifyResourceCommon (
   EFI_STRING                                   EndOfChar;
   REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
 
-  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
+  Supported = RedfishIdentifyResource (Private->Uri, Json);
   if (Supported) {
     Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION, REDPATH_ARRAY_PATTERN, &ConfigLangList);
     if (EFI_ERROR (Status)) {
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115688): https://edk2.groups.io/g/devel/message/115688
Mute This Topic: https://groups.io/mt/104479574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/4] RedfishClientPkg/RedfishFeatureUtilityLib: fix memory leak on error path

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

One comment below,

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Igor Kulchytskyy
> <igork@ami.com>; Nickle Wang <nicklew@nvidia.com>; Chang, Abner
> <Abner.Chang@amd.com>
> Subject: [PATCH 1/4] RedfishClientPkg/RedfishFeatureUtilityLib: fix memory
> leak on error path
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> Reviewed-by: Abner Chang <abner.chang@amd.com>
I think my name should be in "Cc:"?
Abner


> ---
>  .../Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c  | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index e1494471038c..21ce8ddad9d5 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -4010,6 +4010,7 @@ RedfishRemoveUnchangeableProperties (
>               (RedfishCS_uint32)AsciiStrSize (*JsonString)
>
>               );
>
>    if (Status != RedfishCS_status_success) {
>
> +    FreePool (UpdatedJsonString);
>
>      return EFI_DEVICE_ERROR;
>
>    }
>
>
>
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115697): https://edk2.groups.io/g/devel/message/115697
Mute This Topic: https://groups.io/mt/104479571/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 3/4] RedfishClientPkg/Bios: fix leak of GetPendingSettings URI.

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Chang, Abner
> <Abner.Chang@amd.com>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: [PATCH 3/4] RedfishClientPkg/Bios: fix leak of GetPendingSettings
> URI.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index 396ec22969b5..8548425c9c83 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -132,6 +132,7 @@ RedfishResourceConsumeResource (
>    // Check and see if "@Redfish.Settings" exist or not.
>
>    //
>
>    ZeroMem (&PendingSettingResponse, sizeof (REDFISH_RESPONSE));
>
> +  PendingSettingUri = NULL;
>
>    Status = GetPendingSettings (
>
>               Private->RedfishService,
>
>               Response.Payload,
>
> @@ -206,6 +207,10 @@ RedfishResourceConsumeResource (
>      FreePool (Etag);
>
>    }
>
>
>
> +  if (PendingSettingUri != NULL) {
>
> +    FreePool (PendingSettingUri);
>
> +  }
>
> +
>
>    return Status;
>
>  }
>
>
>
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115698): https://edk2.groups.io/g/devel/message/115698
Mute This Topic: https://groups.io/mt/104479573/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 4/4] RedfishClientPkg: use Json value from a function argument

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Looks good, thank you!

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Chang, Abner
> <Abner.Chang@amd.com>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: [PATCH 4/4] RedfishClientPkg: use Json value from a function
> argument
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> This patch replaces value Private->Json with Json used as second argument
> for RedfishIdentifyResource(). Currently Json argument is not used at all
> and the pattern for caller side is:
>   Status = RedfishIdentifyResourceCommon (Private, Private->Json);
>
> So in scope of RedfishIdentifyResourceCommon Json actually is the same
> value as Private->Json. Let's make code a bit cleaner.
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c       | 2 +-
>  .../Features/BootOption/v1_0_4/Common/BootOptionCommon.c        | 2 +-
>  .../ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c        | 2
> +-
>  .../ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c         | 2
> +-
>  RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index 0ae841499692..f3f993c8782e 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -729,7 +729,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
>
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
>
>
>
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
>
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
>
>    if (Supported) {
>
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
>
>      if (EFI_ERROR (Status)) {
>
> diff --git
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> index 0b9f2bf28434..f471c01c3790 100644
> ---
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> +++
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> @@ -791,7 +791,7 @@ RedfishIdentifyResourceCommon (
>  {
>
>    BOOLEAN  Supported;
>
>
>
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
>
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
>
>    if (Supported) {
>
>      return EFI_SUCCESS;
>
>    }
>
> diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/Compute
> rSystemCommon.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/Compute
> rSystemCommon.c
> index cee6c8bf9ba1..d69fc176ad94 100644
> ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/Compute
> rSystemCommon.c
> +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/Compute
> rSystemCommon.c
> @@ -848,7 +848,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
>
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
>
>
>
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
>
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
>
>    if (Supported) {
>
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
>
>      if (EFI_ERROR (Status)) {
>
> diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> index a67ef3dac283..11bcb5f76cab 100644
> ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> @@ -1718,7 +1718,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
>
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
>
>
>
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
>
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
>
>    if (Supported) {
>
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
>
>      if (EFI_ERROR (Status)) {
>
> diff --git
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> c
> index eb52c68c5dcb..00a69f748c3c 100644
> ---
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> +++
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> c
> @@ -2516,7 +2516,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
>
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
>
>
>
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
>
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
>
>    if (Supported) {
>
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
>
>      if (EFI_ERROR (Status)) {
>
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115699): https://edk2.groups.io/g/devel/message/115699
Mute This Topic: https://groups.io/mt/104479574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Nickle Wang
> <nicklew@nvidia.com>; Chang, Abner <Abner.Chang@amd.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: [PATCH 2/4] RedfishClientPkg: refine
> RedfishExternalResourceResourceFeatureCallback
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Use local variable for CollectionUri passed to HandleResource() to avoid
> problems in case of Private->Uri is overriden down the call stack.
>
> Suggested-by: Nickle Wang <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index f40f2d85af80..396ec22969b5 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -670,6 +670,7 @@ RedfishExternalResourceResourceFeatureCallback (
>    REDFISH_SERVICE                  RedfishService;
>
>    REDFISH_RESOURCE_COMMON_PRIVATE  *Private;
>
>    EFI_STRING                       ResourceUri;
>
> +  EFI_STRING                       CollectionUri;
>
>
>
>    if (FeatureAction != CallbackActionStartOperation) {
>
>      return EFI_UNSUPPORTED;
>
> @@ -707,19 +708,19 @@ RedfishExternalResourceResourceFeatureCallback
> (
>    //
>
>    // Initialize collection path
>
>    //
>
> -  Private->Uri = RedfishGetUri (ResourceUri);
>
> -  if (Private->Uri == NULL) {
>
> +  CollectionUri = RedfishGetUri (ResourceUri);

I would like to leave this to Nickle to review if there is any impacts of not initializing Private->Uri. Is Private->Uri referred in the later process?
Apart from above, the naming of CollectionUri is not proper as BiosDxe is not a collection driver. I think the comment of " // Initialize collection path" is a copy & paste error.

Thanks
Abner

>
> +  if (CollectionUri == NULL) {
>
>      ASSERT (FALSE);
>
>      FreePool (ResourceUri);
>
>      return EFI_OUT_OF_RESOURCES;
>
>    }
>
>
>
> -  Status = HandleResource (Private, Private->Uri);
>
> +  Status = HandleResource (Private, CollectionUri);
>
>    if (EFI_ERROR (Status)) {
>
> -    DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n",
> __func__, Private->Uri, Status));
>
> +    DEBUG ((DEBUG_ERROR, "%a, process external resource: %s failed: %r\n",
> __func__, CollectionUri, Status));
>
>    }
>
>
>
> -  FreePool (Private->Uri);
>
> +  FreePool (CollectionUri);
>
>    FreePool (ResourceUri);
>
>    return Status;
>
>  }
>
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115700): https://edk2.groups.io/g/devel/message/115700
Mute This Topic: https://groups.io/mt/104479572/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/4] RedfishClientPkg/RedfishFeatureUtilityLib: fix memory leak on error path

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Igor Kulchytskyy
> <igork@ami.com>; Nickle Wang <nicklew@nvidia.com>; Abner Chang
> <abner.chang@amd.com>
> Subject: [PATCH 1/4] RedfishClientPkg/RedfishFeatureUtilityLib: fix memory leak
> on error path
> 
> External email: Use caution opening links or attachments
> 
> 
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> Reviewed-by: Abner Chang <abner.chang@amd.com>
> ---
>  .../Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c  | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> index e1494471038c..21ce8ddad9d5 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> @@ -4010,6 +4010,7 @@ RedfishRemoveUnchangeableProperties (
>               (RedfishCS_uint32)AsciiStrSize (*JsonString)
> 
>               );
> 
>    if (Status != RedfishCS_status_success) {
> 
> +    FreePool (UpdatedJsonString);
> 
>      return EFI_DEVICE_ERROR;
> 
>    }
> 
> 
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115715): https://edk2.groups.io/g/devel/message/115715
Mute This Topic: https://groups.io/mt/104479571/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

[Nickle Wang via groups.io]

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

Hi Mike,



Thanks for incorporating my suggestion to address memory issue. For the name "CollectionUri", I know this is from my suggestion, but I think Abner is right. Could you please change it to "BiosUri" or "ResourceUri" since BIOS resource is not a Redfish collection?



@Abner Chang<mailto:abner.chang@amd.com> please comment here if you have other naming preference.



Thanks,

Nickle



> -----Original Message-----

> From: Mike Maslenkin <mike.maslenkin@gmail.com>

> Sent: Wednesday, February 21, 2024 8:13 AM

> To: devel@edk2.groups.io

> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Nickle Wang

> <nicklew@nvidia.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy

> <igork@ami.com>

> Subject: [PATCH 2/4] RedfishClientPkg: refine

> RedfishExternalResourceResourceFeatureCallback

>

> External email: Use caution opening links or attachments

>

>

> Use local variable for CollectionUri passed to HandleResource() to avoid problems

> in case of Private->Uri is overriden down the call stack.

>

> Suggested-by: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>

> Cc: Abner Chang <abner.chang@amd.com<mailto:abner.chang@amd.com>>

> Cc: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>

> Cc: Igor Kulchytskyy <igork@ami.com<mailto:igork@ami.com>>

> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>

> ---

>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)

>

> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> index f40f2d85af80..396ec22969b5 100644

> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> @@ -670,6 +670,7 @@ RedfishExternalResourceResourceFeatureCallback (

>    REDFISH_SERVICE                  RedfishService;

>

>    REDFISH_RESOURCE_COMMON_PRIVATE  *Private;

>

>    EFI_STRING                       ResourceUri;

>

> +  EFI_STRING                       CollectionUri;

>

>

>

>    if (FeatureAction != CallbackActionStartOperation) {

>

>      return EFI_UNSUPPORTED;

>

> @@ -707,19 +708,19 @@ RedfishExternalResourceResourceFeatureCallback (

>    //

>

>    // Initialize collection path

>

>    //

>

> -  Private->Uri = RedfishGetUri (ResourceUri);

>

> -  if (Private->Uri == NULL) {

>

> +  CollectionUri = RedfishGetUri (ResourceUri);

>

> +  if (CollectionUri == NULL) {

>

>      ASSERT (FALSE);

>

>      FreePool (ResourceUri);

>

>      return EFI_OUT_OF_RESOURCES;

>

>    }

>

>

>

> -  Status = HandleResource (Private, Private->Uri);

>

> +  Status = HandleResource (Private, CollectionUri);

>

>    if (EFI_ERROR (Status)) {

>

> -    DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n",

> __func__, Private->Uri, Status));

>

> +    DEBUG ((DEBUG_ERROR, "%a, process external resource: %s failed:

> + %r\n", __func__, CollectionUri, Status));

>

>    }

>

>

>

> -  FreePool (Private->Uri);

>

> +  FreePool (CollectionUri);

>

>    FreePool (ResourceUri);

>

>    return Status;

>

>  }

>

> --

> 2.32.0 (Apple Git-132)




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115716): https://edk2.groups.io/g/devel/message/115716
Mute This Topic: https://groups.io/mt/104479572/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

Re: [edk2-devel] [PATCH 3/4] RedfishClientPkg/Bios: fix leak of GetPendingSettings URI.

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Mike
> Maslenkin via groups.io
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Abner Chang
> <abner.chang@amd.com>; Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy
> <igork@ami.com>
> Subject: [edk2-devel] [PATCH 3/4] RedfishClientPkg/Bios: fix leak of
> GetPendingSettings URI.
> 
> External email: Use caution opening links or attachments
> 
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index 396ec22969b5..8548425c9c83 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -132,6 +132,7 @@ RedfishResourceConsumeResource (
>    // Check and see if "@Redfish.Settings" exist or not.
> 
>    //
> 
>    ZeroMem (&PendingSettingResponse, sizeof (REDFISH_RESPONSE));
> 
> +  PendingSettingUri = NULL;
> 
>    Status = GetPendingSettings (
> 
>               Private->RedfishService,
> 
>               Response.Payload,
> 
> @@ -206,6 +207,10 @@ RedfishResourceConsumeResource (
>      FreePool (Etag);
> 
>    }
> 
> 
> 
> +  if (PendingSettingUri != NULL) {
> 
> +    FreePool (PendingSettingUri);
> 
> +  }
> 
> +
> 
>    return Status;
> 
>  }
> 
> 
> 
> --
> 2.32.0 (Apple Git-132)
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#115687): https://edk2.groups.io/g/devel/message/115687
> Mute This Topic: https://groups.io/mt/104479573/7129762
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [nicklew@nvidia.com] -=-=-
> =-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115717): https://edk2.groups.io/g/devel/message/115717
Mute This Topic: https://groups.io/mt/104479573/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 4/4] RedfishClientPkg: use Json value from a function argument

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, February 21, 2024 8:13 AM
> To: devel@edk2.groups.io
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Abner Chang
> <abner.chang@amd.com>; Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy
> <igork@ami.com>
> Subject: [PATCH 4/4] RedfishClientPkg: use Json value from a function argument
> 
> External email: Use caution opening links or attachments
> 
> 
> This patch replaces value Private->Json with Json used as second argument for
> RedfishIdentifyResource(). Currently Json argument is not used at all and the
> pattern for caller side is:
>   Status = RedfishIdentifyResourceCommon (Private, Private->Json);
> 
> So in scope of RedfishIdentifyResourceCommon Json actually is the same value as
> Private->Json. Let's make code a bit cleaner.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c       | 2 +-
>  .../Features/BootOption/v1_0_4/Common/BootOptionCommon.c        | 2 +-
>  .../ComputerSystem/v1_13_0/Common/ComputerSystemCommon.c        | 2 +-
>  .../ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c         | 2 +-
>  RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c   | 2
> +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index 0ae841499692..f3f993c8782e 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -729,7 +729,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
> 
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
> 
> 
> 
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
> 
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
> 
>    if (Supported) {
> 
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
> 
>      if (EFI_ERROR (Status)) {
> 
> diff --git
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.
> c
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.
> c
> index 0b9f2bf28434..f471c01c3790 100644
> ---
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.
> c
> +++
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommo
> +++ n.c
> @@ -791,7 +791,7 @@ RedfishIdentifyResourceCommon (  {
> 
>    BOOLEAN  Supported;
> 
> 
> 
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
> 
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
> 
>    if (Supported) {
> 
>      return EFI_SUCCESS;
> 
>    }
> 
> diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSyst
> emCommon.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSyst
> emCommon.c
> index cee6c8bf9ba1..d69fc176ad94 100644
> ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSyst
> emCommon.c
> +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_13_0/Common/ComputerSy
> +++ stemCommon.c
> @@ -848,7 +848,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
> 
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
> 
> 
> 
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
> 
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
> 
>    if (Supported) {
> 
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
> 
>      if (EFI_ERROR (Status)) {
> 
> diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSyste
> mCommon.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSyste
> mCommon.c
> index a67ef3dac283..11bcb5f76cab 100644
> ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSyste
> mCommon.c
> +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSys
> +++ temCommon.c
> @@ -1718,7 +1718,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
> 
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
> 
> 
> 
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
> 
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
> 
>    if (Supported) {
> 
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
> 
>      if (EFI_ERROR (Status)) {
> 
> diff --git
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> index eb52c68c5dcb..00a69f748c3c 100644
> --- a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> +++ b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> @@ -2516,7 +2516,7 @@ RedfishIdentifyResourceCommon (
>    EFI_STRING                                   EndOfChar;
> 
>    REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG_LIST  ConfigLangList;
> 
> 
> 
> -  Supported = RedfishIdentifyResource (Private->Uri, Private->Json);
> 
> +  Supported = RedfishIdentifyResource (Private->Uri, Json);
> 
>    if (Supported) {
> 
>      Status = RedfishFeatureGetUnifiedArrayTypeConfigureLang
> (RESOURCE_SCHEMA, RESOURCE_SCHEMA_VERSION,
> REDPATH_ARRAY_PATTERN, &ConfigLangList);
> 
>      if (EFI_ERROR (Status)) {
> 
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115718): https://edk2.groups.io/g/devel/message/115718
Mute This Topic: https://groups.io/mt/104479574/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

[Chang, Abner via groups.io]

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

[AMD Official Use Only - General]

Hi Nickle,
I have no problem with either one, maybe BiosUri is a better choice.

Thanks
Abner

From: Nickle Wang <nicklew@nvidia.com>
Sent: Wednesday, February 21, 2024 3:30 PM
To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Subject: RE: [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Hi Mike,



Thanks for incorporating my suggestion to address memory issue. For the name "CollectionUri", I know this is from my suggestion, but I think Abner is right. Could you please change it to "BiosUri" or "ResourceUri" since BIOS resource is not a Redfish collection?



@Abner Chang<mailto:abner.chang@amd.com> please comment here if you have other naming preference.



Thanks,

Nickle



> -----Original Message-----

> From: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>

> Sent: Wednesday, February 21, 2024 8:13 AM

> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>

> Cc: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>; Nickle Wang

> <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>; Abner Chang <abner.chang@amd.com<mailto:abner.chang@amd.com>>; Igor Kulchytskyy

> <igork@ami.com<mailto:igork@ami.com>>

> Subject: [PATCH 2/4] RedfishClientPkg: refine

> RedfishExternalResourceResourceFeatureCallback

>

> External email: Use caution opening links or attachments

>

>

> Use local variable for CollectionUri passed to HandleResource() to avoid problems

> in case of Private->Uri is overriden down the call stack.

>

> Suggested-by: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>

> Cc: Abner Chang <abner.chang@amd.com<mailto:abner.chang@amd.com>>

> Cc: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>

> Cc: Igor Kulchytskyy <igork@ami.com<mailto:igork@ami.com>>

> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com<mailto:mike.maslenkin@gmail.com>>

> ---

>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)

>

> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> index f40f2d85af80..396ec22969b5 100644

> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c

> @@ -670,6 +670,7 @@ RedfishExternalResourceResourceFeatureCallback (

>    REDFISH_SERVICE                  RedfishService;

>

>    REDFISH_RESOURCE_COMMON_PRIVATE  *Private;

>

>    EFI_STRING                       ResourceUri;

>

> +  EFI_STRING                       CollectionUri;

>

>

>

>    if (FeatureAction != CallbackActionStartOperation) {

>

>      return EFI_UNSUPPORTED;

>

> @@ -707,19 +708,19 @@ RedfishExternalResourceResourceFeatureCallback (

>    //

>

>    // Initialize collection path

>

>    //

>

> -  Private->Uri = RedfishGetUri (ResourceUri);

>

> -  if (Private->Uri == NULL) {

>

> +  CollectionUri = RedfishGetUri (ResourceUri);

>

> +  if (CollectionUri == NULL) {

>

>      ASSERT (FALSE);

>

>      FreePool (ResourceUri);

>

>      return EFI_OUT_OF_RESOURCES;

>

>    }

>

>

>

> -  Status = HandleResource (Private, Private->Uri);

>

> +  Status = HandleResource (Private, CollectionUri);

>

>    if (EFI_ERROR (Status)) {

>

> -    DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n",

> __func__, Private->Uri, Status));

>

> +    DEBUG ((DEBUG_ERROR, "%a, process external resource: %s failed:

> + %r\n", __func__, CollectionUri, Status));

>

>    }

>

>

>

> -  FreePool (Private->Uri);

>

> +  FreePool (CollectionUri);

>

>    FreePool (ResourceUri);

>

>    return Status;

>

>  }

>

> --

> 2.32.0 (Apple Git-132)




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115729): https://edk2.groups.io/g/devel/message/115729
Mute This Topic: https://groups.io/mt/104479572/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

Re: [edk2-devel] [PATCH 2/4] RedfishClientPkg: refine RedfishExternalResourceResourceFeatureCallback

[Mike Maslenkin]

On Wed, Feb 21, 2024 at 5:34 AM Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Wednesday, February 21, 2024 8:13 AM
> > To: devel@edk2.groups.io
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Nickle Wang
> > <nicklew@nvidia.com>; Chang, Abner <Abner.Chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>
> > Subject: [PATCH 2/4] RedfishClientPkg: refine
> > RedfishExternalResourceResourceFeatureCallback
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Use local variable for CollectionUri passed to HandleResource() to avoid
> > problems in case of Private->Uri is overriden down the call stack.
> >
> > Suggested-by: Nickle Wang <nicklew@nvidia.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > index f40f2d85af80..396ec22969b5 100644
> > --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > @@ -670,6 +670,7 @@ RedfishExternalResourceResourceFeatureCallback (
> >    REDFISH_SERVICE                  RedfishService;
> >
> >    REDFISH_RESOURCE_COMMON_PRIVATE  *Private;
> >
> >    EFI_STRING                       ResourceUri;
> >
> > +  EFI_STRING                       CollectionUri;
> >
> >
> >
> >    if (FeatureAction != CallbackActionStartOperation) {
> >
> >      return EFI_UNSUPPORTED;
> >
> > @@ -707,19 +708,19 @@ RedfishExternalResourceResourceFeatureCallback
> > (
> >    //
> >
> >    // Initialize collection path
> >
> >    //
> >
> > -  Private->Uri = RedfishGetUri (ResourceUri);
> >
> > -  if (Private->Uri == NULL) {
> >
> > +  CollectionUri = RedfishGetUri (ResourceUri);
>
> I would like to leave this to Nickle to review if there is any impacts of not initializing Private->Uri. Is Private->Uri referred in the later process?
> Apart from above, the naming of CollectionUri is not proper as BiosDxe is not a collection driver. I think the comment of " // Initialize collection path" is a copy & paste error.
>
> Thanks
> Abner
>

Good question about Private->Uri.
It looks like it's safe not to initialize Private->Uri here, because
it is being initialized in every function of
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL.
That was exactly my previous concern.  After this patch only callee
from EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL are dealing with
Private->Uri.
So it could be dropped in favor of additional parameter.

I mean currently EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL function
initializes Private->Uri explicitly, then it is used in function
implementing actual implementation of
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL. For example:
RedfishResourceConsumeResource->RedfishConsumeResourceCommon,
RedfishResourceConsumeResource->RedfishConsumeResourceCommon,
RedfishResourceProvisioningResource->RedfishProvisioningResourceCommon->{ProvisioningBiosExistResource,ProvisioningBiosResources},
RedfishResourceCheck->RedfishCheckResourceCommon,
RedfishResourceUpdate->RedfishUpdateResourceCommon,
RedfishResourceIdentify->RedfishIdentifyResourceCommon.
So, in all this cases Private->Uri can be passed as a function
argument down to call stack.


And HandleResource() mentioned above calls functions from
EdkIIRedfishResourceConfigLib that in turn calls
EDKII_REDFISH_RESOURCE_CONFIG_PROTOCOL implementation, where
Private->Uri is being initialized.  See previous paragraph.

I don't think I missed anything.

Regards,
Mike.


> >
> > +  if (CollectionUri == NULL) {
> >
> >      ASSERT (FALSE);
> >
> >      FreePool (ResourceUri);
> >
> >      return EFI_OUT_OF_RESOURCES;
> >
> >    }
> >
> >
> >
> > -  Status = HandleResource (Private, Private->Uri);
> >
> > +  Status = HandleResource (Private, CollectionUri);
> >
> >    if (EFI_ERROR (Status)) {
> >
> > -    DEBUG ((DEBUG_ERROR, "%a, process external resource: %a failed: %r\n",
> > __func__, Private->Uri, Status));
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, process external resource: %s failed: %r\n",
> > __func__, CollectionUri, Status));
> >
> >    }
> >
> >
> >
> > -  FreePool (Private->Uri);
> >
> > +  FreePool (CollectionUri);
> >
> >    FreePool (ResourceUri);
> >
> >    return Status;
> >
> >  }
> >
> > --
> > 2.32.0 (Apple Git-132)
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115732): https://edk2.groups.io/g/devel/message/115732
Mute This Topic: https://groups.io/mt/104479572/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-