* [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:16 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments Mike Maslenkin
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__
2023-09-29 21:59 ` [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__ Mike Maslenkin
@ 2023-10-02 2:16 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:16 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
2023-09-29 21:59 ` [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__ Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:16 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing Mike Maslenkin
` (7 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments
2023-09-29 21:59 ` [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments Mike Maslenkin
@ 2023-10-02 2:16 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:16 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
2023-09-29 21:59 ` [edk2-devel] [PATCH 1/9] RedfishClientPkg: fix format used for output __func__ Mike Maslenkin
2023-09-29 21:59 ` [edk2-devel] [PATCH 2/9] RedfishClientPkg: fix DEBUG macro arguments Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:18 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks Mike Maslenkin
` (6 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing
2023-09-29 21:59 ` [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing Mike Maslenkin
@ 2023-10-02 2:18 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:18 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (2 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 3/9] RedfishLib: remove redudant zeroing Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:25 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return Mike Maslenkin
` (5 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks
2023-09-29 21:59 ` [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks Mike Maslenkin
@ 2023-10-02 2:25 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:25 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (3 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 4/9] RedfishClientPkg: RedfishFeatureUtilityLib: fix memory leaks Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 3:00 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings Mike Maslenkin
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
2023-09-29 21:59 ` [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return Mike Maslenkin
@ 2023-10-02 3:00 ` Chang, Abner via groups.io
2023-10-02 3:09 ` Chang, Abner via groups.io
0 siblings, 1 reply; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 3:00 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
2023-10-02 3:00 ` Chang, Abner via groups.io
@ 2023-10-02 3:09 ` Chang, Abner via groups.io
2023-10-11 1:53 ` Nickle Wang via groups.io
0 siblings, 1 reply; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 3:09 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io, nicklew@nvidia.com; +Cc: igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
2023-10-02 3:09 ` Chang, Abner via groups.io
@ 2023-10-11 1:53 ` Nickle Wang via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Nickle Wang via groups.io @ 2023-10-11 1:53 UTC (permalink / raw)
To: Chang, Abner, Mike Maslenkin, devel@edk2.groups.io; +Cc: igork@ami.com
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (4 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:30 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak Mike Maslenkin
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings
2023-09-29 21:59 ` [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings Mike Maslenkin
@ 2023-10-02 2:30 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:30 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (5 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 6/9] RedfishClientPkg: fix memory leaks while applying feature settings Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:36 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage Mike Maslenkin
` (2 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak
2023-09-29 21:59 ` [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak Mike Maslenkin
@ 2023-10-02 2:36 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:36 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (6 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 7/9] RedfishClientPkg: fix memory leak Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:45 ` Chang, Abner via groups.io
2023-10-03 2:02 ` Chang, Abner via groups.io
2023-09-29 21:59 ` [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments Mike Maslenkin
2023-10-26 8:33 ` [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Nickle Wang via groups.io
9 siblings, 2 replies; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
2023-09-29 21:59 ` [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage Mike Maslenkin
@ 2023-10-02 2:45 ` Chang, Abner via groups.io
2023-10-02 12:14 ` Mike Maslenkin
2023-10-11 1:49 ` Nickle Wang via groups.io
2023-10-03 2:02 ` Chang, Abner via groups.io
1 sibling, 2 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:45 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io, nicklew@nvidia.com; +Cc: igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
2023-10-02 2:45 ` Chang, Abner via groups.io
@ 2023-10-02 12:14 ` Mike Maslenkin
2023-10-11 1:49 ` Nickle Wang via groups.io
1 sibling, 0 replies; 27+ messages in thread
From: Mike Maslenkin @ 2023-10-02 12:14 UTC (permalink / raw)
To: Chang, Abner; +Cc: devel@edk2.groups.io, nicklew@nvidia.com, igork@ami.com
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
2023-10-02 2:45 ` Chang, Abner via groups.io
2023-10-02 12:14 ` Mike Maslenkin
@ 2023-10-11 1:49 ` Nickle Wang via groups.io
1 sibling, 0 replies; 27+ messages in thread
From: Nickle Wang via groups.io @ 2023-10-11 1:49 UTC (permalink / raw)
To: Chang, Abner, Mike Maslenkin, devel@edk2.groups.io; +Cc: igork@ami.com
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage
2023-09-29 21:59 ` [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage Mike Maslenkin
2023-10-02 2:45 ` Chang, Abner via groups.io
@ 2023-10-03 2:02 ` Chang, Abner via groups.io
1 sibling, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-03 2:02 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (7 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 8/9] RedfishClientPkg: fix pragma pack usage Mike Maslenkin
@ 2023-09-29 21:59 ` Mike Maslenkin
2023-10-02 2:56 ` Chang, Abner via groups.io
2023-10-26 8:33 ` [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Nickle Wang via groups.io
9 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-09-29 21:59 UTC (permalink / raw)
To: devel; +Cc: abner.chang, nicklew, igork, 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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
2023-09-29 21:59 ` [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments Mike Maslenkin
@ 2023-10-02 2:56 ` Chang, Abner via groups.io
2023-10-26 23:51 ` Mike Maslenkin
0 siblings, 1 reply; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-02 2:56 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
2023-10-02 2:56 ` Chang, Abner via groups.io
@ 2023-10-26 23:51 ` Mike Maslenkin
2023-10-27 2:33 ` Chang, Abner via groups.io
0 siblings, 1 reply; 27+ messages in thread
From: Mike Maslenkin @ 2023-10-26 23:51 UTC (permalink / raw)
To: Chang, Abner; +Cc: devel@edk2.groups.io, nicklew@nvidia.com, igork@ami.com
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments
2023-10-26 23:51 ` Mike Maslenkin
@ 2023-10-27 2:33 ` Chang, Abner via groups.io
0 siblings, 0 replies; 27+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-27 2:33 UTC (permalink / raw)
To: Mike Maslenkin; +Cc: devel@edk2.groups.io, nicklew@nvidia.com, igork@ami.com
[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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes
2023-09-29 21:59 [edk2-devel] [PATCH 0/9] RedfishClientPkg: various minor fixes Mike Maslenkin
` (8 preceding siblings ...)
2023-09-29 21:59 ` [edk2-devel] [PATCH 9/9] RedfishClientPkg: fix StrnCpyS arguments Mike Maslenkin
@ 2023-10-26 8:33 ` Nickle Wang via groups.io
9 siblings, 0 replies; 27+ messages in thread
From: Nickle Wang via groups.io @ 2023-10-26 8:33 UTC (permalink / raw)
To: Mike Maslenkin, devel@edk2.groups.io; +Cc: abner.chang@amd.com, igork@ami.com
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread