[edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes

[edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes

[Mike Maslenkin]

This patchset contains fixes of wrong format and number of arguments
passed to DEBUG macro.

Also a number of memory leaks were fixed.

Here is a link to PR:
https://github.com/tianocore/edk2-redfish-client/pull/52

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>



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

[edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__

[Mike Maslenkin]

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index a969557ddfdb..f96c90cc945c 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -788,7 +788,7 @@ HandleResource (
   // Check and see if this is target resource that we want to handle.
   // Some resource is handled by other provider so we have to make sure this first.
   //
-  DEBUG ((REDFISH_DEBUG_TRACE, "%s Identify for %s\n", __func__, Uri));
+  DEBUG ((REDFISH_DEBUG_TRACE, "%a Identify for %s\n", __func__, Uri));
   ConfigLang = RedfishGetConfigLanguage (Uri);
   if (ConfigLang == NULL) {
     Status = EdkIIRedfishResourceConfigIdentify (&SchemaInfo, Uri, Private->InformationExchange);
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments

[Mike Maslenkin]

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib.c                         | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 1e5c3f110de6..35e342c817b7 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -332,7 +332,7 @@ ApplyFeatureSettingsStringType (
         DEBUG ((DEBUG_ERROR, "%a, apply %s to %s failed: %r\n", __func__, ConfigureLang, FeatureValue, Status));
       }
     } else {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %s\n", __func__, Schema, Version, ConfigureLang, RedfishValue.Value.Buffer, Status));
+      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureLang, RedfishValue.Value.Buffer));
     }
   }
 
@@ -462,7 +462,7 @@ ApplyFeatureSettingsBooleanType (
         DEBUG ((DEBUG_ERROR, "%a, apply %s to %a failed: %r\n", __func__, ConfigureLang, (FeatureValue ? "True" : "False"), Status));
       }
     } else {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureLang, (RedfishValue.Value.Boolean ? "True" : "False"), Status));
+      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureLang, (RedfishValue.Value.Boolean ? "True" : "False")));
     }
   }
 
@@ -585,7 +585,7 @@ ApplyFeatureSettingsVagueType (
             DEBUG ((DEBUG_ERROR, "%a, apply %a to %a failed: %r\n", __func__, ConfigureKeyLang, CurrentVagueValuePtr->Value->DataValue.CharPtr, Status));
           }
         } else {
-          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Buffer, Status));
+          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Buffer));
         }
       } else if (PropertyDatatype == RedfishValueTypeBoolean) {
         //
@@ -617,7 +617,7 @@ ApplyFeatureSettingsVagueType (
             DEBUG ((DEBUG_ERROR, "%a, apply %s to %a failed: %r\n", __func__, ConfigureKeyLang, (*CurrentVagueValuePtr->Value->DataValue.BoolPtr ? "True" : "False"), Status));
           }
         } else {
-          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureKeyLang, (RedfishValue.Value.Boolean ? "True" : "False"), Status));
+          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n", __func__, Schema, Version, ConfigureKeyLang, (RedfishValue.Value.Boolean ? "True" : "False")));
         }
       } else if (PropertyDatatype == RedfishValueTypeInteger) {
         //
@@ -640,7 +640,7 @@ ApplyFeatureSettingsVagueType (
             DEBUG ((DEBUG_ERROR, "%a, apply %s to 0x%x failed: %r\n", __func__, ConfigureKeyLang, *CurrentVagueValuePtr->Value->DataValue.Int64Ptr, Status));
           }
         } else {
-          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: 0x%x\n", __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Integer, Status));
+          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: 0x%x\n", __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Integer));
         }
       } else {
         DEBUG ((DEBUG_ERROR, "%a, %a.%a %s Unsupported Redfish property data type\n", __func__, Schema, Version, ConfigureLang));
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing

[Mike Maslenkin]

Memory allocated by calloc() is filled with bytes of value zero.

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../PrivateLibrary/RedfishLib/edk2libredfish/src/service.c       | 1 -
 1 file changed, 1 deletion(-)

diff --git a/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c b/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
index a38bdfbea65f..0ffc23725d37 100644
--- a/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
+++ b/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
@@ -1773,7 +1773,6 @@ createServiceEnumeratorNoAuth (
   char            *HostStart;
 
   ret = (redfishService *)calloc (1, sizeof (redfishService));
-  ZeroMem (ret, sizeof (redfishService));
   if (initRest (ret, restProtocol) != 0) {
     free (ret);
     return NULL;
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks

[Mike Maslenkin]

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib.c                | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 35e342c817b7..8fa1dc2c3535 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -515,6 +515,7 @@ ApplyFeatureSettingsVagueType (
   Status = UnicodeStrToAsciiStrS (ConfigureLang, ConfigureLangAscii, StrLen (ConfigureLang) + 1);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, Convert the configureLang of vague key of %a.%a %s failed: %r\n", __func__, Schema, Version, ConfigureLang, Status));
+    FreePool (ConfigureLangAscii);
     return Status;
   }
 
@@ -1972,6 +1973,7 @@ RedfishGetUri (
     //
     if (*Target == '\0') {
       DEBUG ((DEBUG_ERROR, "%a, invalid format: %s\n", __func__, ConfigLang));
+      FreePool (ResultStr);
       return NULL;
     }
 
@@ -1983,6 +1985,7 @@ RedfishGetUri (
     TempStrSize  = (ConfigLangLen - RemainingLen + 1) * sizeof (CHAR16);
     TempStr      = AllocateCopyPool (TempStrSize, Head);
     if (TempStr == NULL) {
+      FreePool (ResultStr);
       return NULL;
     }
 
@@ -1996,6 +1999,8 @@ RedfishGetUri (
                                        );
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "%a, Can not find: %s\n", __func__, TempStr));
+      FreePool (ResultStr);
+      FreePool (TempStr);
       return NULL;
     }
 
@@ -2102,10 +2107,14 @@ GetConfigureLang (
 
   Status = AsciiStrToUnicodeStrS (Uri, UnicodeUri, StringSize);
   if (EFI_ERROR (Status)) {
+    FreePool (UnicodeUri);
     return NULL;
   }
 
   ConfigLang = RedfishGetConfigLanguage (UnicodeUri);
+
+  FreePool (UnicodeUri);
+
   if (ConfigLang == NULL) {
     return NULL;
   }
@@ -2117,11 +2126,14 @@ GetConfigureLang (
   StringSize = StrSize (ConfigLang) + ((AsciiStrLen (PropertyName) + 1) * sizeof (CHAR16));
   ResultStr  = AllocatePool (StringSize);
   if (ResultStr == NULL) {
+    FreePool (ConfigLang);
     return NULL;
   }
 
   UnicodeSPrint (ResultStr, StringSize, L"%s/%a", ConfigLang, PropertyName);
 
+  FreePool (ConfigLang);
+
   return ResultStr;
 }
 
@@ -2296,9 +2308,12 @@ GetPropertyStringValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeString) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
@@ -2354,9 +2369,12 @@ GetPropertyNumericValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeInteger) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not numeric type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
@@ -2416,9 +2434,12 @@ GetPropertyBooleanValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeBoolean) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not boolean type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
@@ -2517,9 +2538,12 @@ GetPropertyStringArrayValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeStringArray) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
@@ -2588,9 +2612,12 @@ GetPropertyNumericArrayValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
@@ -2659,9 +2686,12 @@ GetPropertyBooleanArrayValue (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLangBuffer, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed: %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
+    FreePool (ConfigureLangBuffer);
     return NULL;
   }
 
+  FreePool (ConfigureLangBuffer);
+
   if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
     return NULL;
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return

[Mike Maslenkin]

This functions contain memory leaks.
Less identation helps to solve this issues.

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib.c                | 289 +++++++++---------
 1 file changed, 146 insertions(+), 143 deletions(-)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 8fa1dc2c3535..0941f33fd73a 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -763,68 +763,69 @@ ApplyFeatureSettingsStringArrayType (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema, Version, ConfigureLang, Status));
-  } else {
-    if (RedfishValue.Type != RedfishValueTypeStringArray) {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
-      return EFI_DEVICE_ERROR;
-    }
+    return Status;
+  }
+
+  if (RedfishValue.Type != RedfishValueTypeStringArray) {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
+    return EFI_DEVICE_ERROR;
+  }
 
+  //
+  // If there is no change in array, do nothing
+  //
+  if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
     //
-    // If there is no change in array, do nothing
+    // Apply settings from redfish
     //
-    if (!CompareRedfishStringArrayValues (ArrayHead, RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
-      //
-      // Apply settings from redfish
-      //
-      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
-      FreeArrayTypeRedfishValue (&RedfishValue);
+    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
+    FreeArrayTypeRedfishValue (&RedfishValue);
 
-      //
-      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
-      //
-      RedfishValue.ArrayCount = 0;
-      Buffer                  = ArrayHead;
-      while (Buffer != NULL) {
-        RedfishValue.ArrayCount += 1;
-        Buffer                   = Buffer->Next;
-      }
+    //
+    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
+    //
+    RedfishValue.ArrayCount = 0;
+    Buffer                  = ArrayHead;
+    while (Buffer != NULL) {
+      RedfishValue.ArrayCount += 1;
+      Buffer                   = Buffer->Next;
+    }
 
-      //
-      // Allocate pool for new values
-      //
-      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
-      if (RedfishValue.Value.StringArray == NULL) {
+    //
+    // Allocate pool for new values
+    //
+    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount *sizeof (CHAR8 *));
+    if (RedfishValue.Value.StringArray == NULL) {
+      ASSERT (FALSE);
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    Buffer = ArrayHead;
+    Index  = 0;
+    while (Buffer != NULL) {
+      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
+      if (RedfishValue.Value.StringArray[Index] == NULL) {
         ASSERT (FALSE);
         return EFI_OUT_OF_RESOURCES;
       }
 
-      Buffer = ArrayHead;
-      Index  = 0;
-      while (Buffer != NULL) {
-        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
-        if (RedfishValue.Value.StringArray[Index] == NULL) {
-          ASSERT (FALSE);
-          return EFI_OUT_OF_RESOURCES;
-        }
-
-        Buffer = Buffer->Next;
-        Index++;
-      }
+      Buffer = Buffer->Next;
+      Index++;
+    }
 
-      ASSERT (Index <= RedfishValue.ArrayCount);
+    ASSERT (Index <= RedfishValue.ArrayCount);
 
-      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
-      if (!EFI_ERROR (Status)) {
-        //
-        // Configuration changed. Enable system reboot flag.
-        //
-        REDFISH_ENABLE_SYSTEM_REBOOT ();
-      } else {
-        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
-      }
+    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
+    if (!EFI_ERROR (Status)) {
+      //
+      // Configuration changed. Enable system reboot flag.
+      //
+      REDFISH_ENABLE_SYSTEM_REBOOT ();
     } else {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
+      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
     }
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
   return Status;
@@ -866,63 +867,64 @@ ApplyFeatureSettingsNumericArrayType (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema, Version, ConfigureLang, Status));
-  } else {
-    if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
-      return EFI_DEVICE_ERROR;
-    }
+    return Status;
+  }
+
+  if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
+    return EFI_DEVICE_ERROR;
+  }
 
+  //
+  // If there is no change in array, do nothing
+  //
+  if (!CompareRedfishNumericArrayValues (ArrayHead, RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
     //
-    // If there is no change in array, do nothing
+    // Apply settings from redfish
     //
-    if (!CompareRedfishNumericArrayValues (ArrayHead, RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
-      //
-      // Apply settings from redfish
-      //
-      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
-      FreeArrayTypeRedfishValue (&RedfishValue);
+    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
+    FreeArrayTypeRedfishValue (&RedfishValue);
 
-      //
-      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
-      //
-      RedfishValue.ArrayCount = 0;
-      Buffer                  = ArrayHead;
-      while (Buffer != NULL) {
-        RedfishValue.ArrayCount += 1;
-        Buffer                   = Buffer->Next;
-      }
+    //
+    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
+    //
+    RedfishValue.ArrayCount = 0;
+    Buffer                  = ArrayHead;
+    while (Buffer != NULL) {
+      RedfishValue.ArrayCount += 1;
+      Buffer                   = Buffer->Next;
+    }
 
-      //
-      // Allocate pool for new values
-      //
-      RedfishValue.Value.IntegerArray = AllocatePool (RedfishValue.ArrayCount * sizeof (INT64));
-      if (RedfishValue.Value.IntegerArray == NULL) {
-        ASSERT (FALSE);
-        return EFI_OUT_OF_RESOURCES;
-      }
+    //
+    // Allocate pool for new values
+    //
+    RedfishValue.Value.IntegerArray = AllocatePool (RedfishValue.ArrayCount * sizeof (INT64));
+    if (RedfishValue.Value.IntegerArray == NULL) {
+      ASSERT (FALSE);
+      return EFI_OUT_OF_RESOURCES;
+    }
 
-      Buffer = ArrayHead;
-      Index  = 0;
-      while (Buffer != NULL) {
-        RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
-        Buffer                                 = Buffer->Next;
-        Index++;
-      }
+    Buffer = ArrayHead;
+    Index  = 0;
+    while (Buffer != NULL) {
+      RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
+      Buffer                                 = Buffer->Next;
+      Index++;
+    }
 
-      ASSERT (Index <= RedfishValue.ArrayCount);
+    ASSERT (Index <= RedfishValue.ArrayCount);
 
-      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
-      if (!EFI_ERROR (Status)) {
-        //
-        // Configuration changed. Enable system reboot flag.
-        //
-        REDFISH_ENABLE_SYSTEM_REBOOT ();
-      } else {
-        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
-      }
+    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
+    if (!EFI_ERROR (Status)) {
+      //
+      // Configuration changed. Enable system reboot flag.
+      //
+      REDFISH_ENABLE_SYSTEM_REBOOT ();
     } else {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
+      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
     }
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
   return Status;
@@ -964,63 +966,64 @@ ApplyFeatureSettingsBooleanArrayType (
   Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang, &RedfishValue);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema, Version, ConfigureLang, Status));
-  } else {
-    if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
-      return EFI_DEVICE_ERROR;
-    }
+    return Status;
+  }
+
+  if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n", __func__, Schema, Version, ConfigureLang));
+    return EFI_DEVICE_ERROR;
+  }
 
+  //
+  // If there is no change in array, do nothing
+  //
+  if (!CompareRedfishBooleanArrayValues (ArrayHead, RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
     //
-    // If there is no change in array, do nothing
+    // Apply settings from redfish
     //
-    if (!CompareRedfishBooleanArrayValues (ArrayHead, RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
-      //
-      // Apply settings from redfish
-      //
-      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
-      FreeArrayTypeRedfishValue (&RedfishValue);
+    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n", __func__, Schema, Version, ConfigureLang));
+    FreeArrayTypeRedfishValue (&RedfishValue);
 
-      //
-      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
-      //
-      RedfishValue.ArrayCount = 0;
-      Buffer                  = ArrayHead;
-      while (Buffer != NULL) {
-        RedfishValue.ArrayCount += 1;
-        Buffer                   = Buffer->Next;
-      }
+    //
+    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
+    //
+    RedfishValue.ArrayCount = 0;
+    Buffer                  = ArrayHead;
+    while (Buffer != NULL) {
+      RedfishValue.ArrayCount += 1;
+      Buffer                   = Buffer->Next;
+    }
 
-      //
-      // Allocate pool for new values
-      //
-      RedfishValue.Value.BooleanArray = AllocatePool (RedfishValue.ArrayCount * sizeof (BOOLEAN));
-      if (RedfishValue.Value.BooleanArray == NULL) {
-        ASSERT (FALSE);
-        return EFI_OUT_OF_RESOURCES;
-      }
+    //
+    // Allocate pool for new values
+    //
+    RedfishValue.Value.BooleanArray = AllocatePool (RedfishValue.ArrayCount * sizeof (BOOLEAN));
+    if (RedfishValue.Value.BooleanArray == NULL) {
+      ASSERT (FALSE);
+      return EFI_OUT_OF_RESOURCES;
+    }
 
-      Buffer = ArrayHead;
-      Index  = 0;
-      while (Buffer != NULL) {
-        RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer->ArrayValue;
-        Buffer                                 = Buffer->Next;
-        Index++;
-      }
+    Buffer = ArrayHead;
+    Index  = 0;
+    while (Buffer != NULL) {
+      RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer->ArrayValue;
+      Buffer                                 = Buffer->Next;
+      Index++;
+    }
 
-      ASSERT (Index <= RedfishValue.ArrayCount);
+    ASSERT (Index <= RedfishValue.ArrayCount);
 
-      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
-      if (!EFI_ERROR (Status)) {
-        //
-        // Configuration changed. Enable system reboot flag.
-        //
-        REDFISH_ENABLE_SYSTEM_REBOOT ();
-      } else {
-        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
-      }
+    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, RedfishValue);
+    if (!EFI_ERROR (Status)) {
+      //
+      // Configuration changed. Enable system reboot flag.
+      //
+      REDFISH_ENABLE_SYSTEM_REBOOT ();
     } else {
-      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
+      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__, ConfigureLang, Status));
     }
+  } else {
+    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
   return Status;
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings

[Mike Maslenkin]

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishFeatureUtilityLib.c                        | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 0941f33fd73a..e189987850f7 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -806,6 +806,7 @@ ApplyFeatureSettingsStringArrayType (
       RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize (Buffer->ArrayValue), Buffer->ArrayValue);
       if (RedfishValue.Value.StringArray[Index] == NULL) {
         ASSERT (FALSE);
+        FreePool (RedfishValue.Value.StringArray);
         return EFI_OUT_OF_RESOURCES;
       }
 
@@ -828,6 +829,12 @@ ApplyFeatureSettingsStringArrayType (
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
+  for (Index = 0; Index < RedfishValue.ArrayCount; Index++) {
+    FreePool (RedfishValue.Value.StringArray[Index]);
+  }
+
+  FreePool (RedfishValue.Value.StringArray);
+
   return Status;
 }
 
@@ -927,6 +934,8 @@ ApplyFeatureSettingsNumericArrayType (
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
+  FreePool (RedfishValue.Value.IntegerArray);
+
   return Status;
 }
 
@@ -1026,6 +1035,8 @@ ApplyFeatureSettingsBooleanArrayType (
     DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n", __func__, Schema, Version, ConfigureLang));
   }
 
+  FreePool (RedfishValue.Value.BooleanArray);
+
   return Status;
 }
 
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak

[Mike Maslenkin]

This patch fixes leak in RedfishExternalResourceResourceFeatureCallback
function.

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index 32dca964aa0a..9b336d3de479 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -718,6 +718,7 @@ RedfishExternalResourceResourceFeatureCallback (
   Private->Uri = RedfishGetUri (ResourceUri);
   if (Private->Uri == NULL) {
     ASSERT (FALSE);
+    FreePool (ResourceUri);
     return EFI_OUT_OF_RESOURCES;
   }
 
@@ -727,6 +728,7 @@ RedfishExternalResourceResourceFeatureCallback (
   }
 
   FreePool (Private->Uri);
+  FreePool (ResourceUri);
   return Status;
 }
 
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage

[Mike Maslenkin]

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
index 7e1bc9cefbac..9d5b10a7e909 100644
--- a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
+++ b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
@@ -31,7 +31,7 @@ extern EFI_GUID  gHiiToRedfishBiosFormsetGuid;
 #define ID_STRING_MAX                  15
 #define ID_STRING_MAX_WITH_TERMINATOR  16
 
-#pragma pack()
+#pragma pack(1)
 
 //
 // Definiton of HII_TO_REDFISH_BIOS_VARSTORE_DATA
@@ -44,4 +44,5 @@ typedef struct {
   UINT8     Reserved;
 } HII_TO_REDFISH_BIOS_EFI_VARSTORE_DATA;
 
+#pragma pack()
 #endif
-- 
2.32.0 (Apple Git-132)



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

[edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments

[Mike Maslenkin]

StrnCpyS accepts string length in characters, not in bytes.

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
index 8ac165dec59e..c19d4a46d6af 100644
--- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
+++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
@@ -331,7 +331,7 @@ NewInternalInstance (
   }
 
   NewInternalData->NodeName = AllocateZeroPool (StrSize (NodeName));
-  StrnCpyS (NewInternalData->NodeName, StrSize (NodeName), (CONST CHAR16 *)NodeName, StrLen (NodeName));
+  StrnCpyS (NewInternalData->NodeName, StrLen (NodeName) + 1, (CONST CHAR16 *)NodeName, StrLen (NodeName));
   NewInternalData->SiblingList = NULL;
   NewInternalData->ChildList   = NULL;
   if (NodeIsCollection) {
-- 
2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 1/9] RedfishClientPkg: fix format used for output __func__
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index a969557ddfdb..f96c90cc945c 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -788,7 +788,7 @@ HandleResource (
>    // Check and see if this is target resource that we want to handle.
>
>    // Some resource is handled by other provider so we have to make sure this
> first.
>
>    //
>
> -  DEBUG ((REDFISH_DEBUG_TRACE, "%s Identify for %s\n", __func__, Uri));
>
> +  DEBUG ((REDFISH_DEBUG_TRACE, "%a Identify for %s\n", __func__, Uri));
>
>    ConfigLang = RedfishGetConfigLanguage (Uri);
>
>    if (ConfigLang == NULL) {
>
>      Status = EdkIIRedfishResourceConfigIdentify (&SchemaInfo, Uri, Private-
> >InformationExchange);
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib.c                         | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index 1e5c3f110de6..35e342c817b7 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -332,7 +332,7 @@ ApplyFeatureSettingsStringType (
>          DEBUG ((DEBUG_ERROR, "%a, apply %s to %s failed: %r\n", __func__,
> ConfigureLang, FeatureValue, Status));
>
>        }
>
>      } else {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %s\n", __func__,
> Schema, Version, ConfigureLang, RedfishValue.Value.Buffer, Status));
>
> +      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__,
> Schema, Version, ConfigureLang, RedfishValue.Value.Buffer));
>
>      }
>
>    }
>
>
>
> @@ -462,7 +462,7 @@ ApplyFeatureSettingsBooleanType (
>          DEBUG ((DEBUG_ERROR, "%a, apply %s to %a failed: %r\n", __func__,
> ConfigureLang, (FeatureValue ? "True" : "False"), Status));
>
>        }
>
>      } else {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__,
> Schema, Version, ConfigureLang, (RedfishValue.Value.Boolean ? "True" :
> "False"), Status));
>
> +      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is: %a\n", __func__,
> Schema, Version, ConfigureLang, (RedfishValue.Value.Boolean ? "True" :
> "False")));
>
>      }
>
>    }
>
>
>
> @@ -585,7 +585,7 @@ ApplyFeatureSettingsVagueType (
>              DEBUG ((DEBUG_ERROR, "%a, apply %a to %a failed: %r\n", __func__,
> ConfigureKeyLang, CurrentVagueValuePtr->Value->DataValue.CharPtr,
> Status));
>
>            }
>
>          } else {
>
> -          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n",
> __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Buffer,
> Status));
>
> +          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n",
> __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Buffer));
>
>          }
>
>        } else if (PropertyDatatype == RedfishValueTypeBoolean) {
>
>          //
>
> @@ -617,7 +617,7 @@ ApplyFeatureSettingsVagueType (
>              DEBUG ((DEBUG_ERROR, "%a, apply %s to %a failed: %r\n", __func__,
> ConfigureKeyLang, (*CurrentVagueValuePtr->Value->DataValue.BoolPtr ?
> "True" : "False"), Status));
>
>            }
>
>          } else {
>
> -          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n",
> __func__, Schema, Version, ConfigureKeyLang, (RedfishValue.Value.Boolean ?
> "True" : "False"), Status));
>
> +          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: %a\n",
> __func__, Schema, Version, ConfigureKeyLang, (RedfishValue.Value.Boolean ?
> "True" : "False")));
>
>          }
>
>        } else if (PropertyDatatype == RedfishValueTypeInteger) {
>
>          //
>
> @@ -640,7 +640,7 @@ ApplyFeatureSettingsVagueType (
>              DEBUG ((DEBUG_ERROR, "%a, apply %s to 0x%x failed: %r\n",
> __func__, ConfigureKeyLang, *CurrentVagueValuePtr->Value-
> >DataValue.Int64Ptr, Status));
>
>            }
>
>          } else {
>
> -          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: 0x%x\n",
> __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Integer,
> Status));
>
> +          DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a %s value is: 0x%x\n",
> __func__, Schema, Version, ConfigureKeyLang, RedfishValue.Value.Integer));
>
>          }
>
>        } else {
>
>          DEBUG ((DEBUG_ERROR, "%a, %a.%a %s Unsupported Redfish property
> data type\n", __func__, Schema, Version, ConfigureLang));
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 3/9] RedfishLib: remove redudant zeroing
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Memory allocated by calloc() is filled with bytes of value zero.
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../PrivateLibrary/RedfishLib/edk2libredfish/src/service.c       | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git
> a/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
> b/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
> index a38bdfbea65f..0ffc23725d37 100644
> --- a/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
> +++
> b/RedfishClientPkg/PrivateLibrary/RedfishLib/edk2libredfish/src/service.c
> @@ -1773,7 +1773,6 @@ createServiceEnumeratorNoAuth (
>    char            *HostStart;
>
>
>
>    ret = (redfishService *)calloc (1, sizeof (redfishService));
>
> -  ZeroMem (ret, sizeof (redfishService));
>
>    if (initRest (ret, restProtocol) != 0) {
>
>      free (ret);
>
>      return NULL;
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory
> leaks
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib.c                | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index 35e342c817b7..8fa1dc2c3535 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -515,6 +515,7 @@ ApplyFeatureSettingsVagueType (
>    Status = UnicodeStrToAsciiStrS (ConfigureLang, ConfigureLangAscii, StrLen
> (ConfigureLang) + 1);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, Convert the configureLang of vague key of
> %a.%a %s failed: %r\n", __func__, Schema, Version, ConfigureLang, Status));
>
> +    FreePool (ConfigureLangAscii);
>
>      return Status;
>
>    }
>
>
>
> @@ -1972,6 +1973,7 @@ RedfishGetUri (
>      //
>
>      if (*Target == '\0') {
>
>        DEBUG ((DEBUG_ERROR, "%a, invalid format: %s\n", __func__,
> ConfigLang));
>
> +      FreePool (ResultStr);
>
>        return NULL;
>
>      }
>
>
>
> @@ -1983,6 +1985,7 @@ RedfishGetUri (
>      TempStrSize  = (ConfigLangLen - RemainingLen + 1) * sizeof (CHAR16);
>
>      TempStr      = AllocateCopyPool (TempStrSize, Head);
>
>      if (TempStr == NULL) {
>
> +      FreePool (ResultStr);
>
>        return NULL;
>
>      }
>
>
>
> @@ -1996,6 +1999,8 @@ RedfishGetUri (
>                                         );
>
>      if (EFI_ERROR (Status)) {
>
>        DEBUG ((DEBUG_ERROR, "%a, Can not find: %s\n", __func__, TempStr));
>
> +      FreePool (ResultStr);
>
> +      FreePool (TempStr);
>
>        return NULL;
>
>      }
>
>
>
> @@ -2102,10 +2107,14 @@ GetConfigureLang (
>
>
>    Status = AsciiStrToUnicodeStrS (Uri, UnicodeUri, StringSize);
>
>    if (EFI_ERROR (Status)) {
>
> +    FreePool (UnicodeUri);
>
>      return NULL;
>
>    }
>
>
>
>    ConfigLang = RedfishGetConfigLanguage (UnicodeUri);
>
> +
>
> +  FreePool (UnicodeUri);
>
> +
>
>    if (ConfigLang == NULL) {
>
>      return NULL;
>
>    }
>
> @@ -2117,11 +2126,14 @@ GetConfigureLang (
>    StringSize = StrSize (ConfigLang) + ((AsciiStrLen (PropertyName) + 1) * sizeof
> (CHAR16));
>
>    ResultStr  = AllocatePool (StringSize);
>
>    if (ResultStr == NULL) {
>
> +    FreePool (ConfigLang);
>
>      return NULL;
>
>    }
>
>
>
>    UnicodeSPrint (ResultStr, StringSize, L"%s/%a", ConfigLang, PropertyName);
>
>
>
> +  FreePool (ConfigLang);
>
> +
>
>    return ResultStr;
>
>  }
>
>
>
> @@ -2296,9 +2308,12 @@ GetPropertyStringValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeString) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> @@ -2354,9 +2369,12 @@ GetPropertyNumericValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeInteger) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not numeric type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> @@ -2416,9 +2434,12 @@ GetPropertyBooleanValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeBoolean) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not boolean type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> @@ -2517,9 +2538,12 @@ GetPropertyStringArrayValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeStringArray) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> @@ -2588,9 +2612,12 @@ GetPropertyNumericArrayValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> @@ -2659,9 +2686,12 @@ GetPropertyBooleanArrayValue (
>    Status = RedfishPlatformConfigGetValue (Schema, Version,
> ConfigureLangBuffer, &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a query current setting for %s failed:
> %r\n", __func__, Schema, Version, ConfigureLangBuffer, Status));
>
> +    FreePool (ConfigureLangBuffer);
>
>      return NULL;
>
>    }
>
>
>
> +  FreePool (ConfigureLangBuffer);
>
> +
>
>    if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
>      return NULL;
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying
> feature settings
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib.c                        | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index 0941f33fd73a..e189987850f7 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -806,6 +806,7 @@ ApplyFeatureSettingsStringArrayType (
>        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> (Buffer->ArrayValue), Buffer->ArrayValue);
>
>        if (RedfishValue.Value.StringArray[Index] == NULL) {
>
>          ASSERT (FALSE);
>
> +        FreePool (RedfishValue.Value.StringArray);
>
>          return EFI_OUT_OF_RESOURCES;
>
>        }
>
>
>
> @@ -828,6 +829,12 @@ ApplyFeatureSettingsStringArrayType (
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
> +  for (Index = 0; Index < RedfishValue.ArrayCount; Index++) {
>
> +    FreePool (RedfishValue.Value.StringArray[Index]);
>
> +  }
>
> +
>
> +  FreePool (RedfishValue.Value.StringArray);
>
> +
>
>    return Status;
>
>  }
>
>
>
> @@ -927,6 +934,8 @@ ApplyFeatureSettingsNumericArrayType (
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
> +  FreePool (RedfishValue.Value.IntegerArray);
>
> +
>
>    return Status;
>
>  }
>
>
>
> @@ -1026,6 +1035,8 @@ ApplyFeatureSettingsBooleanArrayType (
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
> +  FreePool (RedfishValue.Value.BooleanArray);
>
> +
>
>    return Status;
>
>  }
>
>
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak

[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: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 7/9] RedfishClientPkg: fix memory leak
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> This patch fixes leak in RedfishExternalResourceResourceFeatureCallback
> function.
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index 32dca964aa0a..9b336d3de479 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -718,6 +718,7 @@ RedfishExternalResourceResourceFeatureCallback (
>    Private->Uri = RedfishGetUri (ResourceUri);
>
>    if (Private->Uri == NULL) {
>
>      ASSERT (FALSE);
>
> +    FreePool (ResourceUri);
>
>      return EFI_OUT_OF_RESOURCES;
>
>    }
>
>
>
> @@ -727,6 +728,7 @@ RedfishExternalResourceResourceFeatureCallback (
>    }
>
>
>
>    FreePool (Private->Uri);
>
> +  FreePool (ResourceUri);
>
>    return Status;
>
>  }
>
>
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> index 7e1bc9cefbac..9d5b10a7e909 100644
> --- a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> +++ b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> @@ -31,7 +31,7 @@ extern EFI_GUID  gHiiToRedfishBiosFormsetGuid;
>  #define ID_STRING_MAX                  15
>
>  #define ID_STRING_MAX_WITH_TERMINATOR  16
>
>
>
> -#pragma pack()
>
> +#pragma pack(1)
Do we need pack(1) for this structure? We can remove  pack directive if we don't need to have packed structure members. @nicklew@nvidia.com, could you please confirm this?

Thanks
Abner


>
>
>
>  //
>
>  // Definiton of HII_TO_REDFISH_BIOS_VARSTORE_DATA
>
> @@ -44,4 +44,5 @@ typedef struct {
>    UINT8     Reserved;
>
>  } HII_TO_REDFISH_BIOS_EFI_VARSTORE_DATA;
>
>
>
> +#pragma pack()
>
>  #endif
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> StrnCpyS accepts string length in characters, not in bytes.
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> index 8ac165dec59e..c19d4a46d6af 100644
> --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> @@ -331,7 +331,7 @@ NewInternalInstance (
>    }
>
>
>
>    NewInternalData->NodeName = AllocateZeroPool (StrSize (NodeName));
>
> -  StrnCpyS (NewInternalData->NodeName, StrSize (NodeName), (CONST
> CHAR16 *)NodeName, StrLen (NodeName));
>
> +  StrnCpyS (NewInternalData->NodeName, StrLen (NodeName) + 1, (CONST
> CHAR16 *)NodeName, StrLen (NodeName));
The original code is already the size of string that includes NULL terminator. However, we should check if NewInternalData->NodeName is NULL or not before copying the string. Mike, could you please help to add this check?

Thanks
Abner

>
>    NewInternalData->SiblingList = NULL;
>
>    NewInternalData->ChildList   = NULL;
>
>    if (NodeIsCollection) {
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi Mike,
I can't apply the entire patch set from either outlook or Group.io. The format of patch on both are looked weird. I can still review the short ones, but the change of 5/9 is a bit long one. Could you please check the patch format and resend 5/9? Thank you.

Abner

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early
> return
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> This functions contain memory leaks.
> Less identation helps to solve this issues.
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishFeatureUtilityLib.c                | 289 +++++++++---------
>  1 file changed, 146 insertions(+), 143 deletions(-)
>
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index 8fa1dc2c3535..0941f33fd73a 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -763,68 +763,69 @@ ApplyFeatureSettingsStringArrayType (
>    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> Version, ConfigureLang, Status));
>
> -  } else {
>
> -    if (RedfishValue.Type != RedfishValueTypeStringArray) {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      return EFI_DEVICE_ERROR;
>
> -    }
>
> +    return Status;
>
> +  }
>
> +
>
> +  if (RedfishValue.Type != RedfishValueTypeStringArray) {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    return EFI_DEVICE_ERROR;
>
> +  }
>
>
>
> +  //
>
> +  // If there is no change in array, do nothing
>
> +  //
>
> +  if (!CompareRedfishStringArrayValues (ArrayHead,
> RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
>
>      //
>
> -    // If there is no change in array, do nothing
>
> +    // Apply settings from redfish
>
>      //
>
> -    if (!CompareRedfishStringArrayValues (ArrayHead,
> RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
>
> -      //
>
> -      // Apply settings from redfish
>
> -      //
>
> -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      FreeArrayTypeRedfishValue (&RedfishValue);
>
> +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    FreeArrayTypeRedfishValue (&RedfishValue);
>
>
>
> -      //
>
> -      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
>
> -      //
>
> -      RedfishValue.ArrayCount = 0;
>
> -      Buffer                  = ArrayHead;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.ArrayCount += 1;
>
> -        Buffer                   = Buffer->Next;
>
> -      }
>
> +    //
>
> +    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
>
> +    //
>
> +    RedfishValue.ArrayCount = 0;
>
> +    Buffer                  = ArrayHead;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.ArrayCount += 1;
>
> +      Buffer                   = Buffer->Next;
>
> +    }
>
>
>
> -      //
>
> -      // Allocate pool for new values
>
> -      //
>
> -      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> *sizeof (CHAR8 *));
>
> -      if (RedfishValue.Value.StringArray == NULL) {
>
> +    //
>
> +    // Allocate pool for new values
>
> +    //
>
> +    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> *sizeof (CHAR8 *));
>
> +    if (RedfishValue.Value.StringArray == NULL) {
>
> +      ASSERT (FALSE);
>
> +      return EFI_OUT_OF_RESOURCES;
>
> +    }
>
> +
>
> +    Buffer = ArrayHead;
>
> +    Index  = 0;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> (Buffer->ArrayValue), Buffer->ArrayValue);
>
> +      if (RedfishValue.Value.StringArray[Index] == NULL) {
>
>          ASSERT (FALSE);
>
>          return EFI_OUT_OF_RESOURCES;
>
>        }
>
>
>
> -      Buffer = ArrayHead;
>
> -      Index  = 0;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> (Buffer->ArrayValue), Buffer->ArrayValue);
>
> -        if (RedfishValue.Value.StringArray[Index] == NULL) {
>
> -          ASSERT (FALSE);
>
> -          return EFI_OUT_OF_RESOURCES;
>
> -        }
>
> -
>
> -        Buffer = Buffer->Next;
>
> -        Index++;
>
> -      }
>
> +      Buffer = Buffer->Next;
>
> +      Index++;
>
> +    }
>
>
>
> -      ASSERT (Index <= RedfishValue.ArrayCount);
>
> +    ASSERT (Index <= RedfishValue.ArrayCount);
>
>
>
> -      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> -      if (!EFI_ERROR (Status)) {
>
> -        //
>
> -        // Configuration changed. Enable system reboot flag.
>
> -        //
>
> -        REDFISH_ENABLE_SYSTEM_REBOOT ();
>
> -      } else {
>
> -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
> -      }
>
> +    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> +    if (!EFI_ERROR (Status)) {
>
> +      //
>
> +      // Configuration changed. Enable system reboot flag.
>
> +      //
>
> +      REDFISH_ENABLE_SYSTEM_REBOOT ();
>
>      } else {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
> +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
>      }
>
> +  } else {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
>    return Status;
>
> @@ -866,63 +867,64 @@ ApplyFeatureSettingsNumericArrayType (
>    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> Version, ConfigureLang, Status));
>
> -  } else {
>
> -    if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      return EFI_DEVICE_ERROR;
>
> -    }
>
> +    return Status;
>
> +  }
>
> +
>
> +  if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    return EFI_DEVICE_ERROR;
>
> +  }
>
>
>
> +  //
>
> +  // If there is no change in array, do nothing
>
> +  //
>
> +  if (!CompareRedfishNumericArrayValues (ArrayHead,
> RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
>
>      //
>
> -    // If there is no change in array, do nothing
>
> +    // Apply settings from redfish
>
>      //
>
> -    if (!CompareRedfishNumericArrayValues (ArrayHead,
> RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
>
> -      //
>
> -      // Apply settings from redfish
>
> -      //
>
> -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      FreeArrayTypeRedfishValue (&RedfishValue);
>
> +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    FreeArrayTypeRedfishValue (&RedfishValue);
>
>
>
> -      //
>
> -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
>
> -      //
>
> -      RedfishValue.ArrayCount = 0;
>
> -      Buffer                  = ArrayHead;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.ArrayCount += 1;
>
> -        Buffer                   = Buffer->Next;
>
> -      }
>
> +    //
>
> +    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
>
> +    //
>
> +    RedfishValue.ArrayCount = 0;
>
> +    Buffer                  = ArrayHead;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.ArrayCount += 1;
>
> +      Buffer                   = Buffer->Next;
>
> +    }
>
>
>
> -      //
>
> -      // Allocate pool for new values
>
> -      //
>
> -      RedfishValue.Value.IntegerArray = AllocatePool (RedfishValue.ArrayCount
> * sizeof (INT64));
>
> -      if (RedfishValue.Value.IntegerArray == NULL) {
>
> -        ASSERT (FALSE);
>
> -        return EFI_OUT_OF_RESOURCES;
>
> -      }
>
> +    //
>
> +    // Allocate pool for new values
>
> +    //
>
> +    RedfishValue.Value.IntegerArray = AllocatePool (RedfishValue.ArrayCount *
> sizeof (INT64));
>
> +    if (RedfishValue.Value.IntegerArray == NULL) {
>
> +      ASSERT (FALSE);
>
> +      return EFI_OUT_OF_RESOURCES;
>
> +    }
>
>
>
> -      Buffer = ArrayHead;
>
> -      Index  = 0;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
>
> -        Buffer                                 = Buffer->Next;
>
> -        Index++;
>
> -      }
>
> +    Buffer = ArrayHead;
>
> +    Index  = 0;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
>
> +      Buffer                                 = Buffer->Next;
>
> +      Index++;
>
> +    }
>
>
>
> -      ASSERT (Index <= RedfishValue.ArrayCount);
>
> +    ASSERT (Index <= RedfishValue.ArrayCount);
>
>
>
> -      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> -      if (!EFI_ERROR (Status)) {
>
> -        //
>
> -        // Configuration changed. Enable system reboot flag.
>
> -        //
>
> -        REDFISH_ENABLE_SYSTEM_REBOOT ();
>
> -      } else {
>
> -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
> -      }
>
> +    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> +    if (!EFI_ERROR (Status)) {
>
> +      //
>
> +      // Configuration changed. Enable system reboot flag.
>
> +      //
>
> +      REDFISH_ENABLE_SYSTEM_REBOOT ();
>
>      } else {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
> +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
>      }
>
> +  } else {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
>    return Status;
>
> @@ -964,63 +966,64 @@ ApplyFeatureSettingsBooleanArrayType (
>    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> &RedfishValue);
>
>    if (EFI_ERROR (Status)) {
>
>      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> Version, ConfigureLang, Status));
>
> -  } else {
>
> -    if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      return EFI_DEVICE_ERROR;
>
> -    }
>
> +    return Status;
>
> +  }
>
> +
>
> +  if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    return EFI_DEVICE_ERROR;
>
> +  }
>
>
>
> +  //
>
> +  // If there is no change in array, do nothing
>
> +  //
>
> +  if (!CompareRedfishBooleanArrayValues (ArrayHead,
> RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
>
>      //
>
> -    // If there is no change in array, do nothing
>
> +    // Apply settings from redfish
>
>      //
>
> -    if (!CompareRedfishBooleanArrayValues (ArrayHead,
> RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
>
> -      //
>
> -      // Apply settings from redfish
>
> -      //
>
> -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> -      FreeArrayTypeRedfishValue (&RedfishValue);
>
> +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> +    FreeArrayTypeRedfishValue (&RedfishValue);
>
>
>
> -      //
>
> -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
>
> -      //
>
> -      RedfishValue.ArrayCount = 0;
>
> -      Buffer                  = ArrayHead;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.ArrayCount += 1;
>
> -        Buffer                   = Buffer->Next;
>
> -      }
>
> +    //
>
> +    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
>
> +    //
>
> +    RedfishValue.ArrayCount = 0;
>
> +    Buffer                  = ArrayHead;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.ArrayCount += 1;
>
> +      Buffer                   = Buffer->Next;
>
> +    }
>
>
>
> -      //
>
> -      // Allocate pool for new values
>
> -      //
>
> -      RedfishValue.Value.BooleanArray = AllocatePool (RedfishValue.ArrayCount
> * sizeof (BOOLEAN));
>
> -      if (RedfishValue.Value.BooleanArray == NULL) {
>
> -        ASSERT (FALSE);
>
> -        return EFI_OUT_OF_RESOURCES;
>
> -      }
>
> +    //
>
> +    // Allocate pool for new values
>
> +    //
>
> +    RedfishValue.Value.BooleanArray = AllocatePool (RedfishValue.ArrayCount
> * sizeof (BOOLEAN));
>
> +    if (RedfishValue.Value.BooleanArray == NULL) {
>
> +      ASSERT (FALSE);
>
> +      return EFI_OUT_OF_RESOURCES;
>
> +    }
>
>
>
> -      Buffer = ArrayHead;
>
> -      Index  = 0;
>
> -      while (Buffer != NULL) {
>
> -        RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> >ArrayValue;
>
> -        Buffer                                 = Buffer->Next;
>
> -        Index++;
>
> -      }
>
> +    Buffer = ArrayHead;
>
> +    Index  = 0;
>
> +    while (Buffer != NULL) {
>
> +      RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> >ArrayValue;
>
> +      Buffer                                 = Buffer->Next;
>
> +      Index++;
>
> +    }
>
>
>
> -      ASSERT (Index <= RedfishValue.ArrayCount);
>
> +    ASSERT (Index <= RedfishValue.ArrayCount);
>
>
>
> -      Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> -      if (!EFI_ERROR (Status)) {
>
> -        //
>
> -        // Configuration changed. Enable system reboot flag.
>
> -        //
>
> -        REDFISH_ENABLE_SYSTEM_REBOOT ();
>
> -      } else {
>
> -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
> -      }
>
> +    Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang,
> RedfishValue);
>
> +    if (!EFI_ERROR (Status)) {
>
> +      //
>
> +      // Configuration changed. Enable system reboot flag.
>
> +      //
>
> +      REDFISH_ENABLE_SYSTEM_REBOOT ();
>
>      } else {
>
> -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
> +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> ConfigureLang, Status));
>
>      }
>
> +  } else {
>
> +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> __func__, Schema, Version, ConfigureLang));
>
>    }
>
>
>
>    return Status;
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Never mind, I see you had created a PR for this. However, we need @nicklew@nvidia.com to check if the code logic is kept the same with this change.

Thank you for helping on this package, Mike.
Abner

> -----Original Message-----
> From: Chang, Abner
> Sent: Monday, October 2, 2023 11:01 AM
> To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io
> Cc: nicklew@nvidia.com; igork@ami.com
> Subject: RE: [PATCH 5/9] RedfishClientPkg: reduce identation level by adding
> early return
>
> Hi Mike,
> I can't apply the entire patch set from either outlook or Group.io. The format
> of patch on both are looked weird. I can still review the short ones, but the
> change of 5/9 is a bit long one. Could you please check the patch format and
> resend 5/9? Thank you.
>
> Abner
>
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Saturday, September 30, 2023 5:59 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > Subject: [PATCH 5/9] RedfishClientPkg: reduce identation level by adding
> early
> > return
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > This functions contain memory leaks.
> > Less identation helps to solve this issues.
> >
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  .../RedfishFeatureUtilityLib.c                | 289 +++++++++---------
> >  1 file changed, 146 insertions(+), 143 deletions(-)
> >
> > diff --git
> >
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> >
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > index 8fa1dc2c3535..0941f33fd73a 100644
> > ---
> >
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > +++
> >
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > @@ -763,68 +763,69 @@ ApplyFeatureSettingsStringArrayType (
> >    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> > &RedfishValue);
> >
> >    if (EFI_ERROR (Status)) {
> >
> >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> > Version, ConfigureLang, Status));
> >
> > -  } else {
> >
> > -    if (RedfishValue.Type != RedfishValueTypeStringArray) {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      return EFI_DEVICE_ERROR;
> >
> > -    }
> >
> > +    return Status;
> >
> > +  }
> >
> > +
> >
> > +  if (RedfishValue.Type != RedfishValueTypeStringArray) {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    return EFI_DEVICE_ERROR;
> >
> > +  }
> >
> >
> >
> > +  //
> >
> > +  // If there is no change in array, do nothing
> >
> > +  //
> >
> > +  if (!CompareRedfishStringArrayValues (ArrayHead,
> > RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> >
> >      //
> >
> > -    // If there is no change in array, do nothing
> >
> > +    // Apply settings from redfish
> >
> >      //
> >
> > -    if (!CompareRedfishStringArrayValues (ArrayHead,
> > RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> >
> > -      //
> >
> > -      // Apply settings from redfish
> >
> > -      //
> >
> > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      FreeArrayTypeRedfishValue (&RedfishValue);
> >
> > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    FreeArrayTypeRedfishValue (&RedfishValue);
> >
> >
> >
> > -      //
> >
> > -      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
> >
> > -      //
> >
> > -      RedfishValue.ArrayCount = 0;
> >
> > -      Buffer                  = ArrayHead;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.ArrayCount += 1;
> >
> > -        Buffer                   = Buffer->Next;
> >
> > -      }
> >
> > +    //
> >
> > +    // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
> >
> > +    //
> >
> > +    RedfishValue.ArrayCount = 0;
> >
> > +    Buffer                  = ArrayHead;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.ArrayCount += 1;
> >
> > +      Buffer                   = Buffer->Next;
> >
> > +    }
> >
> >
> >
> > -      //
> >
> > -      // Allocate pool for new values
> >
> > -      //
> >
> > -      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> > *sizeof (CHAR8 *));
> >
> > -      if (RedfishValue.Value.StringArray == NULL) {
> >
> > +    //
> >
> > +    // Allocate pool for new values
> >
> > +    //
> >
> > +    RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> > *sizeof (CHAR8 *));
> >
> > +    if (RedfishValue.Value.StringArray == NULL) {
> >
> > +      ASSERT (FALSE);
> >
> > +      return EFI_OUT_OF_RESOURCES;
> >
> > +    }
> >
> > +
> >
> > +    Buffer = ArrayHead;
> >
> > +    Index  = 0;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> > (Buffer->ArrayValue), Buffer->ArrayValue);
> >
> > +      if (RedfishValue.Value.StringArray[Index] == NULL) {
> >
> >          ASSERT (FALSE);
> >
> >          return EFI_OUT_OF_RESOURCES;
> >
> >        }
> >
> >
> >
> > -      Buffer = ArrayHead;
> >
> > -      Index  = 0;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> > (Buffer->ArrayValue), Buffer->ArrayValue);
> >
> > -        if (RedfishValue.Value.StringArray[Index] == NULL) {
> >
> > -          ASSERT (FALSE);
> >
> > -          return EFI_OUT_OF_RESOURCES;
> >
> > -        }
> >
> > -
> >
> > -        Buffer = Buffer->Next;
> >
> > -        Index++;
> >
> > -      }
> >
> > +      Buffer = Buffer->Next;
> >
> > +      Index++;
> >
> > +    }
> >
> >
> >
> > -      ASSERT (Index <= RedfishValue.ArrayCount);
> >
> > +    ASSERT (Index <= RedfishValue.ArrayCount);
> >
> >
> >
> > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > -      if (!EFI_ERROR (Status)) {
> >
> > -        //
> >
> > -        // Configuration changed. Enable system reboot flag.
> >
> > -        //
> >
> > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> > -      } else {
> >
> > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> > -      }
> >
> > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > +    if (!EFI_ERROR (Status)) {
> >
> > +      //
> >
> > +      // Configuration changed. Enable system reboot flag.
> >
> > +      //
> >
> > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> >      } else {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> >      }
> >
> > +  } else {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> >    }
> >
> >
> >
> >    return Status;
> >
> > @@ -866,63 +867,64 @@ ApplyFeatureSettingsNumericArrayType (
> >    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> > &RedfishValue);
> >
> >    if (EFI_ERROR (Status)) {
> >
> >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> > Version, ConfigureLang, Status));
> >
> > -  } else {
> >
> > -    if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      return EFI_DEVICE_ERROR;
> >
> > -    }
> >
> > +    return Status;
> >
> > +  }
> >
> > +
> >
> > +  if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    return EFI_DEVICE_ERROR;
> >
> > +  }
> >
> >
> >
> > +  //
> >
> > +  // If there is no change in array, do nothing
> >
> > +  //
> >
> > +  if (!CompareRedfishNumericArrayValues (ArrayHead,
> > RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
> >
> >      //
> >
> > -    // If there is no change in array, do nothing
> >
> > +    // Apply settings from redfish
> >
> >      //
> >
> > -    if (!CompareRedfishNumericArrayValues (ArrayHead,
> > RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
> >
> > -      //
> >
> > -      // Apply settings from redfish
> >
> > -      //
> >
> > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      FreeArrayTypeRedfishValue (&RedfishValue);
> >
> > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    FreeArrayTypeRedfishValue (&RedfishValue);
> >
> >
> >
> > -      //
> >
> > -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> >
> > -      //
> >
> > -      RedfishValue.ArrayCount = 0;
> >
> > -      Buffer                  = ArrayHead;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.ArrayCount += 1;
> >
> > -        Buffer                   = Buffer->Next;
> >
> > -      }
> >
> > +    //
> >
> > +    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> >
> > +    //
> >
> > +    RedfishValue.ArrayCount = 0;
> >
> > +    Buffer                  = ArrayHead;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.ArrayCount += 1;
> >
> > +      Buffer                   = Buffer->Next;
> >
> > +    }
> >
> >
> >
> > -      //
> >
> > -      // Allocate pool for new values
> >
> > -      //
> >
> > -      RedfishValue.Value.IntegerArray = AllocatePool
> (RedfishValue.ArrayCount
> > * sizeof (INT64));
> >
> > -      if (RedfishValue.Value.IntegerArray == NULL) {
> >
> > -        ASSERT (FALSE);
> >
> > -        return EFI_OUT_OF_RESOURCES;
> >
> > -      }
> >
> > +    //
> >
> > +    // Allocate pool for new values
> >
> > +    //
> >
> > +    RedfishValue.Value.IntegerArray = AllocatePool (RedfishValue.ArrayCount
> *
> > sizeof (INT64));
> >
> > +    if (RedfishValue.Value.IntegerArray == NULL) {
> >
> > +      ASSERT (FALSE);
> >
> > +      return EFI_OUT_OF_RESOURCES;
> >
> > +    }
> >
> >
> >
> > -      Buffer = ArrayHead;
> >
> > -      Index  = 0;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
> >
> > -        Buffer                                 = Buffer->Next;
> >
> > -        Index++;
> >
> > -      }
> >
> > +    Buffer = ArrayHead;
> >
> > +    Index  = 0;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
> >
> > +      Buffer                                 = Buffer->Next;
> >
> > +      Index++;
> >
> > +    }
> >
> >
> >
> > -      ASSERT (Index <= RedfishValue.ArrayCount);
> >
> > +    ASSERT (Index <= RedfishValue.ArrayCount);
> >
> >
> >
> > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > -      if (!EFI_ERROR (Status)) {
> >
> > -        //
> >
> > -        // Configuration changed. Enable system reboot flag.
> >
> > -        //
> >
> > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> > -      } else {
> >
> > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> > -      }
> >
> > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > +    if (!EFI_ERROR (Status)) {
> >
> > +      //
> >
> > +      // Configuration changed. Enable system reboot flag.
> >
> > +      //
> >
> > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> >      } else {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> >      }
> >
> > +  } else {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> >    }
> >
> >
> >
> >    return Status;
> >
> > @@ -964,63 +966,64 @@ ApplyFeatureSettingsBooleanArrayType (
> >    Status = RedfishPlatformConfigGetValue (Schema, Version, ConfigureLang,
> > &RedfishValue);
> >
> >    if (EFI_ERROR (Status)) {
> >
> >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__, Schema,
> > Version, ConfigureLang, Status));
> >
> > -  } else {
> >
> > -    if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      return EFI_DEVICE_ERROR;
> >
> > -    }
> >
> > +    return Status;
> >
> > +  }
> >
> > +
> >
> > +  if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array type\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    return EFI_DEVICE_ERROR;
> >
> > +  }
> >
> >
> >
> > +  //
> >
> > +  // If there is no change in array, do nothing
> >
> > +  //
> >
> > +  if (!CompareRedfishBooleanArrayValues (ArrayHead,
> > RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
> >
> >      //
> >
> > -    // If there is no change in array, do nothing
> >
> > +    // Apply settings from redfish
> >
> >      //
> >
> > -    if (!CompareRedfishBooleanArrayValues (ArrayHead,
> > RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
> >
> > -      //
> >
> > -      // Apply settings from redfish
> >
> > -      //
> >
> > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > -      FreeArrayTypeRedfishValue (&RedfishValue);
> >
> > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +    FreeArrayTypeRedfishValue (&RedfishValue);
> >
> >
> >
> > -      //
> >
> > -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> >
> > -      //
> >
> > -      RedfishValue.ArrayCount = 0;
> >
> > -      Buffer                  = ArrayHead;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.ArrayCount += 1;
> >
> > -        Buffer                   = Buffer->Next;
> >
> > -      }
> >
> > +    //
> >
> > +    // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> >
> > +    //
> >
> > +    RedfishValue.ArrayCount = 0;
> >
> > +    Buffer                  = ArrayHead;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.ArrayCount += 1;
> >
> > +      Buffer                   = Buffer->Next;
> >
> > +    }
> >
> >
> >
> > -      //
> >
> > -      // Allocate pool for new values
> >
> > -      //
> >
> > -      RedfishValue.Value.BooleanArray = AllocatePool
> (RedfishValue.ArrayCount
> > * sizeof (BOOLEAN));
> >
> > -      if (RedfishValue.Value.BooleanArray == NULL) {
> >
> > -        ASSERT (FALSE);
> >
> > -        return EFI_OUT_OF_RESOURCES;
> >
> > -      }
> >
> > +    //
> >
> > +    // Allocate pool for new values
> >
> > +    //
> >
> > +    RedfishValue.Value.BooleanArray = AllocatePool
> (RedfishValue.ArrayCount
> > * sizeof (BOOLEAN));
> >
> > +    if (RedfishValue.Value.BooleanArray == NULL) {
> >
> > +      ASSERT (FALSE);
> >
> > +      return EFI_OUT_OF_RESOURCES;
> >
> > +    }
> >
> >
> >
> > -      Buffer = ArrayHead;
> >
> > -      Index  = 0;
> >
> > -      while (Buffer != NULL) {
> >
> > -        RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> > >ArrayValue;
> >
> > -        Buffer                                 = Buffer->Next;
> >
> > -        Index++;
> >
> > -      }
> >
> > +    Buffer = ArrayHead;
> >
> > +    Index  = 0;
> >
> > +    while (Buffer != NULL) {
> >
> > +      RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> > >ArrayValue;
> >
> > +      Buffer                                 = Buffer->Next;
> >
> > +      Index++;
> >
> > +    }
> >
> >
> >
> > -      ASSERT (Index <= RedfishValue.ArrayCount);
> >
> > +    ASSERT (Index <= RedfishValue.ArrayCount);
> >
> >
> >
> > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > -      if (!EFI_ERROR (Status)) {
> >
> > -        //
> >
> > -        // Configuration changed. Enable system reboot flag.
> >
> > -        //
> >
> > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> > -      } else {
> >
> > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> > -      }
> >
> > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> ConfigureLang,
> > RedfishValue);
> >
> > +    if (!EFI_ERROR (Status)) {
> >
> > +      //
> >
> > +      // Configuration changed. Enable system reboot flag.
> >
> > +      //
> >
> > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> >
> >      } else {
> >
> > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > ConfigureLang, Status));
> >
> >      }
> >
> > +  } else {
> >
> > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> >    }
> >
> >
> >
> >    return Status;
> >
> > --
> > 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage

[Mike Maslenkin]

On Mon, Oct 2, 2023 at 5:45 AM Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Saturday, September 30, 2023 5:59 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > Subject: [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > index 7e1bc9cefbac..9d5b10a7e909 100644
> > --- a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > +++ b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > @@ -31,7 +31,7 @@ extern EFI_GUID  gHiiToRedfishBiosFormsetGuid;
> >  #define ID_STRING_MAX                  15
> >
> >  #define ID_STRING_MAX_WITH_TERMINATOR  16
> >
> >
> >
> > -#pragma pack()
> >
> > +#pragma pack(1)
> Do we need pack(1) for this structure? We can remove  pack directive if we don't need to have packed structure members. @nicklew@nvidia.com, could you please confirm this?
>
> Thanks
> Abner
>
>
> >
> >
> >
> >  //
> >
> >  // Definiton of HII_TO_REDFISH_BIOS_VARSTORE_DATA
> >
> > @@ -44,4 +44,5 @@ typedef struct {
> >    UINT8     Reserved;
> >
> >  } HII_TO_REDFISH_BIOS_EFI_VARSTORE_DATA;
> >
> >
> >
> > +#pragma pack()
> >
> >  #endif
> >
> > --
> > 2.32.0 (Apple Git-132)
>

This structure is stored in EFI variables, so it can be consumed by
code built by another compiler.
And this structure does not have natural alignment of its fields.


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

Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

That makes sense.

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

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> index 7e1bc9cefbac..9d5b10a7e909 100644
> --- a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> +++ b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> @@ -31,7 +31,7 @@ extern EFI_GUID  gHiiToRedfishBiosFormsetGuid;
>  #define ID_STRING_MAX                  15
>
>  #define ID_STRING_MAX_WITH_TERMINATOR  16
>
>
>
> -#pragma pack()
>
> +#pragma pack(1)
>
>
>
>  //
>
>  // Definiton of HII_TO_REDFISH_BIOS_VARSTORE_DATA
>
> @@ -44,4 +44,5 @@ typedef struct {
>    UINT8     Reserved;
>
>  } HII_TO_REDFISH_BIOS_EFI_VARSTORE_DATA;
>
>
>
> +#pragma pack()
>
>  #endif
>
> --
> 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage

[Nickle Wang via groups.io]

Sorry for my late response.

> Do we need pack(1) for this structure? We can remove  pack directive if we don't
> need to have packed structure members. @nicklew@nvidia.com, could you
> please confirm this?

I see the use of "pack(1)" in EDK2 DriverSample.h. I think we can keep it.


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

Regards,
Nickle

> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Monday, October 2, 2023 10:45 AM
> To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io; Nickle
> Wang <nicklew@nvidia.com>
> Cc: igork@ami.com
> Subject: RE: [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
> 
> External email: Use caution opening links or attachments
> 
> 
> [AMD Official Use Only - General]
> 
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Saturday, September 30, 2023 5:59 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > Subject: [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > index 7e1bc9cefbac..9d5b10a7e909 100644
> > --- a/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > +++ b/RedfishClientPkg/HiiToRedfishBiosDxe/HiiToRedfishBiosData.h
> > @@ -31,7 +31,7 @@ extern EFI_GUID  gHiiToRedfishBiosFormsetGuid;
> >  #define ID_STRING_MAX                  15
> >
> >  #define ID_STRING_MAX_WITH_TERMINATOR  16
> >
> >
> >
> > -#pragma pack()
> >
> > +#pragma pack(1)
> Do we need pack(1) for this structure? We can remove  pack directive if we don't
> need to have packed structure members. @nicklew@nvidia.com, could you
> please confirm this?
> 
> Thanks
> Abner
> 
> 
> >
> >
> >
> >  //
> >
> >  // Definiton of HII_TO_REDFISH_BIOS_VARSTORE_DATA
> >
> > @@ -44,4 +44,5 @@ typedef struct {
> >    UINT8     Reserved;
> >
> >  } HII_TO_REDFISH_BIOS_EFI_VARSTORE_DATA;
> >
> >
> >
> > +#pragma pack()
> >
> >  #endif
> >
> > --
> > 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return

[Nickle Wang via groups.io]

Sorry for the delay review.

I go through the changes, and it looks good to me.


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

Regards,
Nickle

> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Monday, October 2, 2023 11:10 AM
> To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io; Nickle
> Wang <nicklew@nvidia.com>
> Cc: igork@ami.com
> Subject: RE: [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early
> return
> 
> External email: Use caution opening links or attachments
> 
> 
> [AMD Official Use Only - General]
> 
> Never mind, I see you had created a PR for this. However, we need
> @nicklew@nvidia.com to check if the code logic is kept the same with this
> change.
> 
> Thank you for helping on this package, Mike.
> Abner
> 
> > -----Original Message-----
> > From: Chang, Abner
> > Sent: Monday, October 2, 2023 11:01 AM
> > To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io
> > Cc: nicklew@nvidia.com; igork@ami.com
> > Subject: RE: [PATCH 5/9] RedfishClientPkg: reduce identation level by
> > adding early return
> >
> > Hi Mike,
> > I can't apply the entire patch set from either outlook or Group.io.
> > The format of patch on both are looked weird. I can still review the
> > short ones, but the change of 5/9 is a bit long one. Could you please
> > check the patch format and resend 5/9? Thank you.
> >
> > Abner
> >
> > > -----Original Message-----
> > > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > Sent: Saturday, September 30, 2023 5:59 AM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > > Subject: [PATCH 5/9] RedfishClientPkg: reduce identation level by
> > > adding
> > early
> > > return
> > >
> > > Caution: This message originated from an External Source. Use proper
> > caution
> > > when opening attachments, clicking links, or responding.
> > >
> > >
> > > This functions contain memory leaks.
> > > Less identation helps to solve this issues.
> > >
> > > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > ---
> > >  .../RedfishFeatureUtilityLib.c                | 289 +++++++++---------
> > >  1 file changed, 146 insertions(+), 143 deletions(-)
> > >
> > > diff --git
> > >
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > >
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > index 8fa1dc2c3535..0941f33fd73a 100644
> > > ---
> > >
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > +++
> > >
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > @@ -763,68 +763,69 @@ ApplyFeatureSettingsStringArrayType (
> > >    Status = RedfishPlatformConfigGetValue (Schema, Version,
> > > ConfigureLang, &RedfishValue);
> > >
> > >    if (EFI_ERROR (Status)) {
> > >
> > >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__,
> > > Schema, Version, ConfigureLang, Status));
> > >
> > > -  } else {
> > >
> > > -    if (RedfishValue.Type != RedfishValueTypeStringArray) {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      return EFI_DEVICE_ERROR;
> > >
> > > -    }
> > >
> > > +    return Status;
> > >
> > > +  }
> > >
> > > +
> > >
> > > +  if (RedfishValue.Type != RedfishValueTypeStringArray) {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > > + type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    return EFI_DEVICE_ERROR;
> > >
> > > +  }
> > >
> > >
> > >
> > > +  //
> > >
> > > +  // If there is no change in array, do nothing
> > >
> > > +  //
> > >
> > > +  if (!CompareRedfishStringArrayValues (ArrayHead,
> > > RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> > >
> > >      //
> > >
> > > -    // If there is no change in array, do nothing
> > >
> > > +    // Apply settings from redfish
> > >
> > >      //
> > >
> > > -    if (!CompareRedfishStringArrayValues (ArrayHead,
> > > RedfishValue.Value.StringArray, RedfishValue.ArrayCount)) {
> > >
> > > -      //
> > >
> > > -      // Apply settings from redfish
> > >
> > > -      //
> > >
> > > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Convert array from RedfishCS_char_Array to EDKII_REDFISH_VALUE
> > >
> > > -      //
> > >
> > > -      RedfishValue.ArrayCount = 0;
> > >
> > > -      Buffer                  = ArrayHead;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.ArrayCount += 1;
> > >
> > > -        Buffer                   = Buffer->Next;
> > >
> > > -      }
> > >
> > > +    //
> > >
> > > +    // Convert array from RedfishCS_char_Array to
> > > + EDKII_REDFISH_VALUE
> > >
> > > +    //
> > >
> > > +    RedfishValue.ArrayCount = 0;
> > >
> > > +    Buffer                  = ArrayHead;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.ArrayCount += 1;
> > >
> > > +      Buffer                   = Buffer->Next;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Allocate pool for new values
> > >
> > > -      //
> > >
> > > -      RedfishValue.Value.StringArray = AllocatePool (RedfishValue.ArrayCount
> > > *sizeof (CHAR8 *));
> > >
> > > -      if (RedfishValue.Value.StringArray == NULL) {
> > >
> > > +    //
> > >
> > > +    // Allocate pool for new values
> > >
> > > +    //
> > >
> > > +    RedfishValue.Value.StringArray = AllocatePool
> > > + (RedfishValue.ArrayCount
> > > *sizeof (CHAR8 *));
> > >
> > > +    if (RedfishValue.Value.StringArray == NULL) {
> > >
> > > +      ASSERT (FALSE);
> > >
> > > +      return EFI_OUT_OF_RESOURCES;
> > >
> > > +    }
> > >
> > > +
> > >
> > > +    Buffer = ArrayHead;
> > >
> > > +    Index  = 0;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.Value.StringArray[Index] = AllocateCopyPool
> > > + (AsciiStrSize
> > > (Buffer->ArrayValue), Buffer->ArrayValue);
> > >
> > > +      if (RedfishValue.Value.StringArray[Index] == NULL) {
> > >
> > >          ASSERT (FALSE);
> > >
> > >          return EFI_OUT_OF_RESOURCES;
> > >
> > >        }
> > >
> > >
> > >
> > > -      Buffer = ArrayHead;
> > >
> > > -      Index  = 0;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.Value.StringArray[Index] = AllocateCopyPool (AsciiStrSize
> > > (Buffer->ArrayValue), Buffer->ArrayValue);
> > >
> > > -        if (RedfishValue.Value.StringArray[Index] == NULL) {
> > >
> > > -          ASSERT (FALSE);
> > >
> > > -          return EFI_OUT_OF_RESOURCES;
> > >
> > > -        }
> > >
> > > -
> > >
> > > -        Buffer = Buffer->Next;
> > >
> > > -        Index++;
> > >
> > > -      }
> > >
> > > +      Buffer = Buffer->Next;
> > >
> > > +      Index++;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > > +    ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > >
> > >
> > > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > -      if (!EFI_ERROR (Status)) {
> > >
> > > -        //
> > >
> > > -        // Configuration changed. Enable system reboot flag.
> > >
> > > -        //
> > >
> > > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > > -      } else {
> > >
> > > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > > ConfigureLang, Status));
> > >
> > > -      }
> > >
> > > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > +    if (!EFI_ERROR (Status)) {
> > >
> > > +      //
> > >
> > > +      // Configuration changed. Enable system reboot flag.
> > >
> > > +      //
> > >
> > > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > >      } else {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n",
> > > + __func__,
> > > ConfigureLang, Status));
> > >
> > >      }
> > >
> > > +  } else {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no
> > > + change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > >    }
> > >
> > >
> > >
> > >    return Status;
> > >
> > > @@ -866,63 +867,64 @@ ApplyFeatureSettingsNumericArrayType (
> > >    Status = RedfishPlatformConfigGetValue (Schema, Version,
> > > ConfigureLang, &RedfishValue);
> > >
> > >    if (EFI_ERROR (Status)) {
> > >
> > >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__,
> > > Schema, Version, ConfigureLang, Status));
> > >
> > > -  } else {
> > >
> > > -    if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      return EFI_DEVICE_ERROR;
> > >
> > > -    }
> > >
> > > +    return Status;
> > >
> > > +  }
> > >
> > > +
> > >
> > > +  if (RedfishValue.Type != RedfishValueTypeIntegerArray) {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > > + type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    return EFI_DEVICE_ERROR;
> > >
> > > +  }
> > >
> > >
> > >
> > > +  //
> > >
> > > +  // If there is no change in array, do nothing
> > >
> > > +  //
> > >
> > > +  if (!CompareRedfishNumericArrayValues (ArrayHead,
> > > RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
> > >
> > >      //
> > >
> > > -    // If there is no change in array, do nothing
> > >
> > > +    // Apply settings from redfish
> > >
> > >      //
> > >
> > > -    if (!CompareRedfishNumericArrayValues (ArrayHead,
> > > RedfishValue.Value.IntegerArray, RedfishValue.ArrayCount)) {
> > >
> > > -      //
> > >
> > > -      // Apply settings from redfish
> > >
> > > -      //
> > >
> > > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> > >
> > > -      //
> > >
> > > -      RedfishValue.ArrayCount = 0;
> > >
> > > -      Buffer                  = ArrayHead;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.ArrayCount += 1;
> > >
> > > -        Buffer                   = Buffer->Next;
> > >
> > > -      }
> > >
> > > +    //
> > >
> > > +    // Convert array from RedfishCS_int64_Array to
> > > + EDKII_REDFISH_VALUE
> > >
> > > +    //
> > >
> > > +    RedfishValue.ArrayCount = 0;
> > >
> > > +    Buffer                  = ArrayHead;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.ArrayCount += 1;
> > >
> > > +      Buffer                   = Buffer->Next;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Allocate pool for new values
> > >
> > > -      //
> > >
> > > -      RedfishValue.Value.IntegerArray = AllocatePool
> > (RedfishValue.ArrayCount
> > > * sizeof (INT64));
> > >
> > > -      if (RedfishValue.Value.IntegerArray == NULL) {
> > >
> > > -        ASSERT (FALSE);
> > >
> > > -        return EFI_OUT_OF_RESOURCES;
> > >
> > > -      }
> > >
> > > +    //
> > >
> > > +    // Allocate pool for new values
> > >
> > > +    //
> > >
> > > +    RedfishValue.Value.IntegerArray = AllocatePool
> > > + (RedfishValue.ArrayCount
> > *
> > > sizeof (INT64));
> > >
> > > +    if (RedfishValue.Value.IntegerArray == NULL) {
> > >
> > > +      ASSERT (FALSE);
> > >
> > > +      return EFI_OUT_OF_RESOURCES;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      Buffer = ArrayHead;
> > >
> > > -      Index  = 0;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.Value.IntegerArray[Index] = (INT64)*Buffer->ArrayValue;
> > >
> > > -        Buffer                                 = Buffer->Next;
> > >
> > > -        Index++;
> > >
> > > -      }
> > >
> > > +    Buffer = ArrayHead;
> > >
> > > +    Index  = 0;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.Value.IntegerArray[Index] =
> > > + (INT64)*Buffer->ArrayValue;
> > >
> > > +      Buffer                                 = Buffer->Next;
> > >
> > > +      Index++;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > > +    ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > >
> > >
> > > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > -      if (!EFI_ERROR (Status)) {
> > >
> > > -        //
> > >
> > > -        // Configuration changed. Enable system reboot flag.
> > >
> > > -        //
> > >
> > > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > > -      } else {
> > >
> > > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > > ConfigureLang, Status));
> > >
> > > -      }
> > >
> > > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > +    if (!EFI_ERROR (Status)) {
> > >
> > > +      //
> > >
> > > +      // Configuration changed. Enable system reboot flag.
> > >
> > > +      //
> > >
> > > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > >      } else {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n",
> > > + __func__,
> > > ConfigureLang, Status));
> > >
> > >      }
> > >
> > > +  } else {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no
> > > + change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > >    }
> > >
> > >
> > >
> > >    return Status;
> > >
> > > @@ -964,63 +966,64 @@ ApplyFeatureSettingsBooleanArrayType (
> > >    Status = RedfishPlatformConfigGetValue (Schema, Version,
> > > ConfigureLang, &RedfishValue);
> > >
> > >    if (EFI_ERROR (Status)) {
> > >
> > >      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s failed: %r\n", __func__,
> > > Schema, Version, ConfigureLang, Status));
> > >
> > > -  } else {
> > >
> > > -    if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      return EFI_DEVICE_ERROR;
> > >
> > > -    }
> > >
> > > +    return Status;
> > >
> > > +  }
> > >
> > > +
> > >
> > > +  if (RedfishValue.Type != RedfishValueTypeBooleanArray) {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s value is not string array
> > > + type\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    return EFI_DEVICE_ERROR;
> > >
> > > +  }
> > >
> > >
> > >
> > > +  //
> > >
> > > +  // If there is no change in array, do nothing
> > >
> > > +  //
> > >
> > > +  if (!CompareRedfishBooleanArrayValues (ArrayHead,
> > > RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
> > >
> > >      //
> > >
> > > -    // If there is no change in array, do nothing
> > >
> > > +    // Apply settings from redfish
> > >
> > >      //
> > >
> > > -    if (!CompareRedfishBooleanArrayValues (ArrayHead,
> > > RedfishValue.Value.BooleanArray, RedfishValue.ArrayCount)) {
> > >
> > > -      //
> > >
> > > -      // Apply settings from redfish
> > >
> > > -      //
> > >
> > > -      DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > -      FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > > +    DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +    FreeArrayTypeRedfishValue (&RedfishValue);
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Convert array from RedfishCS_int64_Array to EDKII_REDFISH_VALUE
> > >
> > > -      //
> > >
> > > -      RedfishValue.ArrayCount = 0;
> > >
> > > -      Buffer                  = ArrayHead;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.ArrayCount += 1;
> > >
> > > -        Buffer                   = Buffer->Next;
> > >
> > > -      }
> > >
> > > +    //
> > >
> > > +    // Convert array from RedfishCS_int64_Array to
> > > + EDKII_REDFISH_VALUE
> > >
> > > +    //
> > >
> > > +    RedfishValue.ArrayCount = 0;
> > >
> > > +    Buffer                  = ArrayHead;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.ArrayCount += 1;
> > >
> > > +      Buffer                   = Buffer->Next;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      //
> > >
> > > -      // Allocate pool for new values
> > >
> > > -      //
> > >
> > > -      RedfishValue.Value.BooleanArray = AllocatePool
> > (RedfishValue.ArrayCount
> > > * sizeof (BOOLEAN));
> > >
> > > -      if (RedfishValue.Value.BooleanArray == NULL) {
> > >
> > > -        ASSERT (FALSE);
> > >
> > > -        return EFI_OUT_OF_RESOURCES;
> > >
> > > -      }
> > >
> > > +    //
> > >
> > > +    // Allocate pool for new values
> > >
> > > +    //
> > >
> > > +    RedfishValue.Value.BooleanArray = AllocatePool
> > (RedfishValue.ArrayCount
> > > * sizeof (BOOLEAN));
> > >
> > > +    if (RedfishValue.Value.BooleanArray == NULL) {
> > >
> > > +      ASSERT (FALSE);
> > >
> > > +      return EFI_OUT_OF_RESOURCES;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      Buffer = ArrayHead;
> > >
> > > -      Index  = 0;
> > >
> > > -      while (Buffer != NULL) {
> > >
> > > -        RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> > > >ArrayValue;
> > >
> > > -        Buffer                                 = Buffer->Next;
> > >
> > > -        Index++;
> > >
> > > -      }
> > >
> > > +    Buffer = ArrayHead;
> > >
> > > +    Index  = 0;
> > >
> > > +    while (Buffer != NULL) {
> > >
> > > +      RedfishValue.Value.BooleanArray[Index] = (BOOLEAN)*Buffer-
> > > >ArrayValue;
> > >
> > > +      Buffer                                 = Buffer->Next;
> > >
> > > +      Index++;
> > >
> > > +    }
> > >
> > >
> > >
> > > -      ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > > +    ASSERT (Index <= RedfishValue.ArrayCount);
> > >
> > >
> > >
> > > -      Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > -      if (!EFI_ERROR (Status)) {
> > >
> > > -        //
> > >
> > > -        // Configuration changed. Enable system reboot flag.
> > >
> > > -        //
> > >
> > > -        REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > > -      } else {
> > >
> > > -        DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n", __func__,
> > > ConfigureLang, Status));
> > >
> > > -      }
> > >
> > > +    Status = RedfishPlatformConfigSetValue (Schema, Version,
> > ConfigureLang,
> > > RedfishValue);
> > >
> > > +    if (!EFI_ERROR (Status)) {
> > >
> > > +      //
> > >
> > > +      // Configuration changed. Enable system reboot flag.
> > >
> > > +      //
> > >
> > > +      REDFISH_ENABLE_SYSTEM_REBOOT ();
> > >
> > >      } else {
> > >
> > > -      DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +      DEBUG ((DEBUG_ERROR, "%a, apply %s array failed: %r\n",
> > > + __func__,
> > > ConfigureLang, Status));
> > >
> > >      }
> > >
> > > +  } else {
> > >
> > > +    DEBUG ((DEBUG_ERROR, "%a, %a.%a %s array value has no
> > > + change\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > >    }
> > >
> > >
> > >
> > >    return Status;
> > >
> > > --
> > > 2.32.0 (Apple Git-132)



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

Re: [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes

[Nickle Wang via groups.io]

Reviewed this patch series.

Hi Mike,

Could you please address Abner's comment on "[PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments"? This patch series can be merged after Abner gives reviewed-by to patch 9/9.

Thanks,
Nickle

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: abner.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
> igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [PATCH 0/9] RedfishClientPkg: various minor fixes
>
> External email: Use caution opening links or attachments
>
>
> This patchset contains fixes of wrong format and number of arguments passed to
> DEBUG macro.
>
> Also a number of memory leaks were fixed.
>
> Here is a link to PR:
> https://github.co/
> m%2Ftianocore%2Fedk2-redfish-
> client%2Fpull%2F52&data=05%7C01%7Cnicklew%40nvidia.com%7Cbd1ef61ecbc
> b4af7b03008dbc13755d1%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0
> %7C638316215640671905%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
> &sdata=pNrDEeRVMbIkXIoLnH55bUn3yC10AseuA5albTZQFHQ%3D&reserved=0
>
> 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>



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

Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments

[Mike Maslenkin]

On Mon, Oct 2, 2023 at 5:56 AM Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Saturday, September 30, 2023 5:59 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > Subject: [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > StrnCpyS accepts string length in characters, not in bytes.
> >
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > index 8ac165dec59e..c19d4a46d6af 100644
> > --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > @@ -331,7 +331,7 @@ NewInternalInstance (
> >    }
> >
> >
> >
> >    NewInternalData->NodeName = AllocateZeroPool (StrSize (NodeName));
> >
> > -  StrnCpyS (NewInternalData->NodeName, StrSize (NodeName), (CONST
> > CHAR16 *)NodeName, StrLen (NodeName));
> >
> > +  StrnCpyS (NewInternalData->NodeName, StrLen (NodeName) + 1, (CONST
> > CHAR16 *)NodeName, StrLen (NodeName));
> The original code is already the size of string that includes NULL terminator. However, we should check if NewInternalData->NodeName is NULL or not before copying the string. Mike, could you please help to add this check?
>
> Thanks
> Abner
>
Hi, Abner

The problem is not with NULL terminator
The problem is that StrnCpyS takes a number of unicode chars as a
second parameter, not a string size in bytes returned by StrSize().

So I left this patch unmodified and added two additional patches
required for NULL pointer check.

Thanks,
Mike.


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

Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, October 27, 2023 7:51 AM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; nicklew@nvidia.com; igork@ami.com
> Subject: Re: [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, Oct 2, 2023 at 5:56 AM Chang, Abner <Abner.Chang@amd.com>
> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > > -----Original Message-----
> > > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > Sent: Saturday, September 30, 2023 5:59 AM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner <Abner.Chang@amd.com>; nicklew@nvidia.com;
> > > igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>
> > > Subject: [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
> > >
> > > Caution: This message originated from an External Source. Use proper
> caution
> > > when opening attachments, clicking links, or responding.
> > >
> > >
> > > StrnCpyS accepts string length in characters, not in bytes.
> > >
> > > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > ---
> > >  RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git
> a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > > b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > > index 8ac165dec59e..c19d4a46d6af 100644
> > > --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > > +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> > > @@ -331,7 +331,7 @@ NewInternalInstance (
> > >    }
> > >
> > >
> > >
> > >    NewInternalData->NodeName = AllocateZeroPool (StrSize (NodeName));
> > >
> > > -  StrnCpyS (NewInternalData->NodeName, StrSize (NodeName), (CONST
> > > CHAR16 *)NodeName, StrLen (NodeName));
> > >
> > > +  StrnCpyS (NewInternalData->NodeName, StrLen (NodeName) + 1,
> (CONST
> > > CHAR16 *)NodeName, StrLen (NodeName));
> > The original code is already the size of string that includes NULL terminator.
> However, we should check if NewInternalData->NodeName is NULL or not
> before copying the string. Mike, could you please help to add this check?
> >
> > Thanks
> > Abner
> >
> Hi, Abner
>
> The problem is not with NULL terminator
> The problem is that StrnCpyS takes a number of unicode chars as a
> second parameter, not a string size in bytes returned by StrSize().
Yeah got it.

>
> So I left this patch unmodified and added two additional patches
> required for NULL pointer check.
Ok

Abner
>
> Thanks,
> Mike.


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