Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
Sorry for the delay review.
I go through the changes, and it looks good to me.
Reviewed-by: Nickle Wang
Regards,
Nickle
> -Original Message-
> From: Chang, Abner
> Sent: Monday, October 2, 2023 11:10 AM
> To: Mike Maslenkin ; devel@edk2.groups.io; Nickle
> Wang
> Cc: ig...@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
> @nick...@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 ; devel@edk2.groups.io
> > Cc: nick...@nvidia.com; ig...@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
> > > Sent: Saturday, September 30, 2023 5:59 AM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner ; nick...@nvidia.com;
> > > ig...@ami.com; Mike Maslenkin
> > > 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
> > > ---
> > > .../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, );
> > >
> > >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 ();
> > >
> > > +DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > > __func__, Schema, Version, ConfigureLang));
> > >
> > > +FreeArrayTypeRedfishValue ();
> > >
> > >
> > >
> > > - //
> > >
> > > - // 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
Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
[AMD Official Use Only - General]
Never mind, I see you had created a PR for this. However, we need
@nick...@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 ; devel@edk2.groups.io
> Cc: nick...@nvidia.com; ig...@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
> > Sent: Saturday, September 30, 2023 5:59 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner ; nick...@nvidia.com;
> > ig...@ami.com; Mike Maslenkin
> > 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
> > ---
> > .../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,
> > );
> >
> >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 ();
> >
> > +DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> > __func__, Schema, Version, ConfigureLang));
> >
> > +FreeArrayTypeRedfishValue ();
> >
> >
> >
> > - //
> >
> > - // 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 ==
Re: [edk2-devel] [PATCH 5/9] RedfishClientPkg: reduce identation level by adding early return
[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
> Sent: Saturday, September 30, 2023 5:59 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; nick...@nvidia.com;
> ig...@ami.com; Mike Maslenkin
> 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
> ---
> .../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,
> );
>
>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 ();
>
> +DEBUG ((DEBUG_MANAGEABILITY, "%a, %a.%a apply %s for array\n",
> __func__, Schema, Version, ConfigureLang));
>
> +FreeArrayTypeRedfishValue ();
>
>
>
> - //
>
> - // 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;
>
> -}
>
> -
>
>
