[PATCH 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard


Ni, Ray
 

The change doesn't impact any functionality.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/CpuDxe/CpuGdt.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c
index a1ab543f2d..322ce87142 100644
--- a/UefiCpuPkg/CpuDxe/CpuGdt.c
+++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
@@ -2,7 +2,7 @@
C based implementation of IA32 interrupt handling only=0D
requiring a minimal assembly interrupt entry point.=0D
=0D
- Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>=0D
+ Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -13,7 +13,7 @@
//=0D
// Global descriptor table (GDT) Template=0D
//=0D
-STATIC GDT_ENTRIES GdtTemplate =3D {=0D
+STATIC GDT_ENTRIES gGdtTemplate =3D {=0D
//=0D
// NULL_SEL=0D
//=0D
@@ -124,32 +124,31 @@ InitGlobalDescriptorTable (
VOID=0D
)=0D
{=0D
- GDT_ENTRIES *gdt;=0D
- IA32_DESCRIPTOR gdtPtr;=0D
+ GDT_ENTRIES *Gdt;=0D
+ IA32_DESCRIPTOR Gdtr;=0D
=0D
//=0D
// Allocate Runtime Data for the GDT=0D
//=0D
- gdt =3D AllocateRuntimePool (sizeof (GdtTemplate) + 8);=0D
- ASSERT (gdt !=3D NULL);=0D
- gdt =3D ALIGN_POINTER (gdt, 8);=0D
+ Gdt =3D AllocateRuntimePool (sizeof (gGdtTemplate) + 8);=0D
+ ASSERT (Gdt !=3D NULL);=0D
+ Gdt =3D ALIGN_POINTER (Gdt, 8);=0D
=0D
//=0D
// Initialize all GDT entries=0D
//=0D
- CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));=0D
+ CopyMem (Gdt, &gGdtTemplate, sizeof (gGdtTemplate));=0D
=0D
//=0D
// Write GDT register=0D
//=0D
- gdtPtr.Base =3D (UINT32)(UINTN)(VOID*) gdt;=0D
- gdtPtr.Limit =3D (UINT16) (sizeof (GdtTemplate) - 1);=0D
- AsmWriteGdtr (&gdtPtr);=0D
+ Gdtr.Base =3D (UINT32) (UINTN) Gdt;=0D
+ Gdtr.Limit =3D (UINT16) (sizeof (gGdtTemplate) - 1);=0D
+ AsmWriteGdtr (&Gdtr);=0D
=0D
//=0D
// Update selector (segment) registers base on new GDT=0D
//=0D
- SetCodeSelector ((UINT16)CPU_CODE_SEL);=0D
- SetDataSelectors ((UINT16)CPU_DATA_SEL);=0D
+ SetCodeSelector ((UINT16) CPU_CODE_SEL);=0D
+ SetDataSelectors ((UINT16) CPU_DATA_SEL);=0D
}=0D
-=0D
--=20
2.27.0.windows.1


Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Tuesday, March 16, 2021 11:34 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Laszlo Ersek <lersek@...>; Kumar, Rahul1 <rahul1.kumar@...>
Subject: [PATCH 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard

The change doesn't impact any functionality.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/CpuDxe/CpuGdt.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c
index a1ab543f2d..322ce87142 100644
--- a/UefiCpuPkg/CpuDxe/CpuGdt.c
+++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
@@ -2,7 +2,7 @@
C based implementation of IA32 interrupt handling only

requiring a minimal assembly interrupt entry point.



- Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -13,7 +13,7 @@
//

// Global descriptor table (GDT) Template

//

-STATIC GDT_ENTRIES GdtTemplate = {

+STATIC GDT_ENTRIES gGdtTemplate = {

//

// NULL_SEL

//

@@ -124,32 +124,31 @@ InitGlobalDescriptorTable (
VOID

)

{

- GDT_ENTRIES *gdt;

- IA32_DESCRIPTOR gdtPtr;

+ GDT_ENTRIES *Gdt;

+ IA32_DESCRIPTOR Gdtr;



//

// Allocate Runtime Data for the GDT

//

- gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);

- ASSERT (gdt != NULL);

- gdt = ALIGN_POINTER (gdt, 8);

+ Gdt = AllocateRuntimePool (sizeof (gGdtTemplate) + 8);

+ ASSERT (Gdt != NULL);

+ Gdt = ALIGN_POINTER (Gdt, 8);



//

// Initialize all GDT entries

//

- CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));

+ CopyMem (Gdt, &gGdtTemplate, sizeof (gGdtTemplate));



//

// Write GDT register

//

- gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;

- gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1);

- AsmWriteGdtr (&gdtPtr);

+ Gdtr.Base = (UINT32) (UINTN) Gdt;

+ Gdtr.Limit = (UINT16) (sizeof (gGdtTemplate) - 1);

+ AsmWriteGdtr (&Gdtr);



//

// Update selector (segment) registers base on new GDT

//

- SetCodeSelector ((UINT16)CPU_CODE_SEL);

- SetDataSelectors ((UINT16)CPU_DATA_SEL);

+ SetCodeSelector ((UINT16) CPU_CODE_SEL);

+ SetDataSelectors ((UINT16) CPU_DATA_SEL);

}

-

--
2.27.0.windows.1


Laszlo Ersek
 

On 03/16/21 04:33, Ray Ni wrote:
The change doesn't impact any functionality.

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
UefiCpuPkg/CpuDxe/CpuGdt.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c
index a1ab543f2d..322ce87142 100644
--- a/UefiCpuPkg/CpuDxe/CpuGdt.c
+++ b/UefiCpuPkg/CpuDxe/CpuGdt.c
@@ -2,7 +2,7 @@
C based implementation of IA32 interrupt handling only
requiring a minimal assembly interrupt entry point.

- Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -13,7 +13,7 @@
//
// Global descriptor table (GDT) Template
//
-STATIC GDT_ENTRIES GdtTemplate = {
+STATIC GDT_ENTRIES gGdtTemplate = {
//
// NULL_SEL
//
@@ -124,32 +124,31 @@ InitGlobalDescriptorTable (
VOID
)
{
- GDT_ENTRIES *gdt;
- IA32_DESCRIPTOR gdtPtr;
+ GDT_ENTRIES *Gdt;
+ IA32_DESCRIPTOR Gdtr;

//
// Allocate Runtime Data for the GDT
//
- gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);
- ASSERT (gdt != NULL);
- gdt = ALIGN_POINTER (gdt, 8);
+ Gdt = AllocateRuntimePool (sizeof (gGdtTemplate) + 8);
+ ASSERT (Gdt != NULL);
+ Gdt = ALIGN_POINTER (Gdt, 8);

//
// Initialize all GDT entries
//
- CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));
+ CopyMem (Gdt, &gGdtTemplate, sizeof (gGdtTemplate));

//
// Write GDT register
//
- gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;
- gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1);
- AsmWriteGdtr (&gdtPtr);
+ Gdtr.Base = (UINT32) (UINTN) Gdt;
+ Gdtr.Limit = (UINT16) (sizeof (gGdtTemplate) - 1);
+ AsmWriteGdtr (&Gdtr);

//
// Update selector (segment) registers base on new GDT
//
- SetCodeSelector ((UINT16)CPU_CODE_SEL);
- SetDataSelectors ((UINT16)CPU_DATA_SEL);
+ SetCodeSelector ((UINT16) CPU_CODE_SEL);
+ SetDataSelectors ((UINT16) CPU_DATA_SEL);
}
-
(1) I think "mGdtTemplate" would be a better name than "gGdtTemplate". I
think the "g" prefix is used when an object is identical for all
firmware modules (such as named GUIDs, for example).

(2) I think the last hunk does not belong in this patch -- more
precisely, I *disagree* with the last hunk. Inserting a space in the
middle of a typecast, after the parenthesized typename, is a bad
practice in edk2; it is error prone and suggests that typecasts have low
binding power (when in reality they have almost the strongest binding).

Thanks
Laszlo


Ni, Ray
 

(1) I think "mGdtTemplate" would be a better name than "gGdtTemplate". I
think the "g" prefix is used when an object is identical for all
firmware modules (such as named GUIDs, for example).
Agree! I will change in v2.



(2) I think the last hunk does not belong in this patch -- more
precisely, I *disagree* with the last hunk. Inserting a space in the
middle of a typecast, after the parenthesized typename, is a bad
practice in edk2; it is error prone and suggests that typecasts have low
binding power (when in reality they have almost the strongest binding).
I double checked the edk2 coding standard and did find a rule for this.
That might be just my personal preference. Since I need your Ack or Rb, I will remove
this change in v2.