From 50383be71b571fc86c0c0179a5d1bff06a59388b Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 13 Sep 2023 01:43:08 +0300 Subject: [PATCH 01/11] RedfishClientPkg: fix format used for output __func__ Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- 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 a969557dd..f96c90cc9 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); From 0a012cc3f9e3cbfa9232b18fb5d9faa99f27627b Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 27 Sep 2023 23:15:03 +0300 Subject: [PATCH 02/11] RedfishClientPkg: fix DEBUG macro arguments Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../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 1e5c3f110..35e342c81 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)); From 20270a2fb09f87a142a5e4713233f74a5ed4ff15 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 27 Sep 2023 23:19:12 +0300 Subject: [PATCH 03/11] RedfishLib: remove redudant zeroing Memory allocated by calloc() is filled with bytes of value zero. Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../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 a38bdfbea..0ffc23725 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; From 7aaf9728214b9beb9db2a108bfc8cbd697ecf37a Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 13 Sep 2023 01:03:28 +0300 Subject: [PATCH 04/11] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../RedfishFeatureUtilityLib.c | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c index 35e342c81..8fa1dc2c3 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; From b698b4aa0d3f3810f4afd9745baf6490ecce6376 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Fri, 29 Sep 2023 00:41:34 +0300 Subject: [PATCH 05/11] RedfishClientPkg: reduce identation level by adding early return This functions contain memory leaks. Less identation helps to solve this issues. Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../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 8fa1dc2c3..0941f33fd 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; From c99cf548fc92904cf4f96e4ddf219f46951f8f61 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Fri, 29 Sep 2023 00:58:05 +0300 Subject: [PATCH 06/11] RedfishClientPkg: fix memory leaks while applying feature settings Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../RedfishFeatureUtilityLib.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c index 0941f33fd..e18998785 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; } From 26026f4a7b112f443433ceab2f2cb562715d98ca Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Thu, 28 Sep 2023 20:34:12 +0300 Subject: [PATCH 07/11] RedfishClientPkg: fix memory leak This patch fixes leak in RedfishExternalResourceResourceFeatureCallback function. Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- 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 32dca964a..9b336d3de 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; } From fcfbe0fee59333f228cca692928b2dbecb679da6 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 27 Sep 2023 23:18:04 +0300 Subject: [PATCH 08/11] RedfishClientPkg: fix pragma pack usage Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- 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 7e1bc9cef..9d5b10a7e 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 From 38ed05bf624eda8738e0025ad5a1ba70811f9fec Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Wed, 27 Sep 2023 23:16:42 +0300 Subject: [PATCH 09/11] RedfishClientPkg: fix StrnCpyS arguments StrnCpyS accepts string length in characters, not in bytes. Reviewed-by: Abner Chang Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- 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 8ac165dec..c19d4a46d 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) { From 2172bf9a2e85febbe7b707e93350e7d832619614 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Fri, 27 Oct 2023 02:06:07 +0300 Subject: [PATCH 10/11] RedfishFeatureCoreDxe: replace __FUNCTION__ with __func__ Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c index c19d4a46d..f71b12e4b 100644 --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c @@ -319,14 +319,14 @@ NewInternalInstance ( REDFISH_FEATURE_INTERNAL_DATA *NewInternalData; if ((PtrToNewInternalData == NULL) || (NodeName == NULL)) { - DEBUG ((DEBUG_ERROR, "%a: Inproper given parameters\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: Inproper given parameters\n", __func__)); return EFI_INVALID_PARAMETER; } *PtrToNewInternalData = NULL; NewInternalData = AllocateZeroPool (sizeof (REDFISH_FEATURE_INTERNAL_DATA)); if (NewInternalData == NULL) { - DEBUG ((DEBUG_ERROR, "%a: No memory for REDFISH_FEATURE_INTERNAL_DATA\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: No memory for REDFISH_FEATURE_INTERNAL_DATA\n", __func__)); return EFI_OUT_OF_RESOURCES; } @@ -377,12 +377,12 @@ InsertRedfishFeatureUriNode ( *MatchNodeEntry = NULL; if (NodeName == NULL) { - DEBUG ((DEBUG_ERROR, "%a: Node name is NULL.\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: Node name is NULL.\n", __func__)); return EFI_INVALID_PARAMETER; } if (NextNodeEntry == NULL) { - DEBUG ((DEBUG_ERROR, "%a: NextNodeEntry can't be NULL.\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: NextNodeEntry can't be NULL.\n", __func__)); return EFI_INVALID_PARAMETER; } @@ -487,7 +487,7 @@ RedfishFeatureRegister ( BOOLEAN ItsCollection; if ((FeatureManagedUri == NULL) || (Callback == NULL)) { - DEBUG ((DEBUG_ERROR, "%a: The given parameter is invalid\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: The given parameter is invalid\n", __func__)); return EFI_INVALID_PARAMETER; } @@ -503,7 +503,7 @@ RedfishFeatureRegister ( while ((Index < UriLength)) { if ((Index - AnchorIndex + 1) >= MaxNodeNameLength) { // Increase one for the NULL terminator - DEBUG ((DEBUG_ERROR, "%a: the length of node name is >= MaxNodeNameLength\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: the length of node name is >= MaxNodeNameLength\n", __func__)); ASSERT (FALSE); } @@ -568,7 +568,7 @@ RedfishFeatureRegister ( // // No URI node was created // - DEBUG ((DEBUG_ERROR, "%a: No URI node is added\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "%a: No URI node is added\n", __func__)); return EFI_INVALID_PARAMETER; } From a905d8442964c40c346490db9857bc38f8dffb79 Mon Sep 17 00:00:00 2001 From: Mike Maslenkin Date: Fri, 27 Oct 2023 02:09:56 +0300 Subject: [PATCH 11/11] RedfishFeatureCoreDxe: add check for memory allocation failure Reviewed-by: Abner Chang Reviewed-by: Nickle Wang Signed-off-by: Mike Maslenkin --- .../RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c index f71b12e4b..d9aa6e1a8 100644 --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c @@ -331,6 +331,12 @@ NewInternalInstance ( } NewInternalData->NodeName = AllocateZeroPool (StrSize (NodeName)); + if (NewInternalData->NodeName == NULL) { + DEBUG ((DEBUG_ERROR, "%a: No memory for NodeName\n", __func__)); + FreePool (NewInternalData); + return EFI_OUT_OF_RESOURCES; + } + StrnCpyS (NewInternalData->NodeName, StrLen (NodeName) + 1, (CONST CHAR16 *)NodeName, StrLen (NodeName)); NewInternalData->SiblingList = NULL; NewInternalData->ChildList = NULL;