Re: [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

["Leif Lindholm" <leif@nuviainc.com>]

On Sun, Feb 21, 2021 at 16:04:31 -0700, Rebecca Cran wrote:
> On 2/21/21 2:21 PM, Leif Lindholm wrote:
> > On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:

(Adding necessary patch context back in - please don't strip out until
absolutely certain it is irrelevant to conversation.)

diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
index e6adbceba2d5..fc4dba319aad 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
@@ -23,27 +23,6 @@

#include "SmbiosMisc.h"

-/**
- * Returns the chassis type in SMBIOS format.
- *
- * @return Chassis type
-**/

> > > -UINT8
> > > -GetChassisType (
> > > -  VOID
> > > -  )
> > > -{
> > > -  EFI_STATUS                      Status;
> > > -  UINT8                           ChassisType;
> > > -
> > > -  Status = OemGetChassisType (&ChassisType);
> > > -  if (EFI_ERROR (Status)) {
> > > -    return 0;
> > > -  }
> > > -
> > > -  return ChassisType;
> > > -}
> > > -
> > 
> > This function is outright deleted, not renamed.
> > Does this belong in another patch?
> 
> I think it belongs here: it was just a helper function for when
> OemGetChassisType returned EFI_STATUS. Since it now returns the same data as
> this function, we don't need it anymore.

This patch, according to its title and commit message, does renaming.

The refactoring should then be moved over to 3/3, together with this
hunk:

@@ -165,10 +142,7 @@
SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)

   SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE3) + SmbiosRecord->ExtendLength + 1;

-  ChassisType = GetChassisType ();
-  if (ChassisType != 0) {
-    SmbiosRecord->Type  = ChassisType;
-  }
+  SmbiosRecord->Type = OemGetChassisType ();

   //ContainedElements
   ASSERT (ContainedElementCount < 2);

The current layout means this code (in 2/3) relies on the API change
introduced in 3/3.

/
    Leif

> 
> -- 
> Rebecca Cran