Re: [edk2-devel] [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

2024-03-12 Thread Zhang, Zifeng
Thanks for sharing.

Best Regards,
Zifeng

-Original Message-
From: gaoliming  
Sent: Friday, March 8, 2024 10:47 PM
To: Zhang, Zifeng ; Yang, Yuting2 
; devel@edk2.groups.io
Cc: 'Rebecca Cran' ; Feng, Bob C ; 
Chen, Christine ; Chen, Arthur G 
Subject: 回复: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

Zifeng:
  For Structure PCD,  https://edk2.groups.io/g/devel/message/18432 is the 
summary of code change. 

  Below is two wikis about its usage. 
  https://github.com/lgao4/edk2/wiki/StrucutrePcd-Usage
  https://github.com/lgao4/edk2/wiki/StructurePcd-Enable-Steps
  
  For the patch table configuration, the structure PCD can be defined to map 
the patch table structure. 
  Then, the patch table value can be specified in DSC in the different SkuId
(boardId) to support multiple SKUs.
  Structure PCD can make DSC as the centralized way for the platform 
configuration. 
  The developer doesn't need to maintain the board setting in C source file.


Thanks
Liming
> -邮件原件-
> 发件人: Zhang, Zifeng 
> 发送时间: 2024年3月7日 14:54
> 收件人: Yang, Yuting2 ; devel@edk2.groups.io
> 抄送: Rebecca Cran ; Liming Gao 
> ; Feng, Bob C ; Chen, 
> Christine ; Chen, Arthur G 
> 
> 主题: RE: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError
> 
> Hi Liming,
> 
> Could you help to review the patch for VFR compiler change submitted 
> by Yuting?
> 
> Btw,
> Dell need some introduction of Hii StructurePCD implementation 
> especially for replacing patch table configurations.
> Would you like to share EDK2 wiki link or doc for it?
> 
> Best Regards,
> Zifeng
> 
> 
> -Original Message-
> From: Yang, Yuting2 
> Sent: Friday, January 26, 2024 10:54 AM
> To: devel@edk2.groups.io
> Cc: Rebecca Cran ; Liming Gao 
> ; Feng, Bob C ; Chen, 
> Christine ; Zhang, Zifeng 
> 
> Subject: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError
> 
> Add --catch_default option to raise a DefaultValueError when 
> encountering VFR default definitions to help remove default variables.
> 
> Signed-off-by: Yuting Yang 
> 
> Cc: Rebecca Cran 
> Cc: Liming Gao 
> Cc: Bob Feng 
> Cc: Christine Chen 
> Cc: Zifeng Zhang 
> Signed-off-by: Yuting Yang 
> ---
>  BaseTools/Source/C/VfrCompile/VfrCompiler.cpp |   8 +-
>  BaseTools/Source/C/VfrCompile/VfrCompiler.h   |   1 +
>  BaseTools/Source/C/VfrCompile/VfrError.cpp|   3 +-
>  BaseTools/Source/C/VfrCompile/VfrError.h  |   3 +-
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.h|   1 +
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 238 ++
>  6 files changed, 150 insertions(+), 104 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> index 5f4d262d85..4031af6e39 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> @@ -78,6 +78,7 @@ CVfrCompiler::OptionInitialization (
>mOptions.WarningAsError= FALSE;
> mOptions.AutoDefault   = FALSE;
> mOptions.CheckDefault  = FALSE;+
> mOptions.IsCatchDefaultEnable  = FALSE;   memset
> (, 0, sizeof (EFI_GUID));if (Argc == 1) {@@
> -95,6 +96,8 @@ CVfrCompiler::OptionInitialization (
>Version ();   SET_RUN_STATUS (STATUS_DEAD);
> return;+} else if (stricmp(Argv[Index], "--catch_default") == 0){+
> mOptions.IsCatchDefaultEnable = TRUE; } else if (stricmp(Argv[Index],
> "-l") == 0) {   mOptions.CreateRecordListFile = TRUE;
> gCIfrRecordInfoDB.TurnOn ();@@ -179,7 +182,6 @@ 
> CVfrCompiler::OptionInitialization (
>goto Fail; } strcpy (mOptions.VfrFileName, Argv[Index]);-
> if (mOptions.OutputDirectory == NULL) {   mOptions.OutputDirectory =
> (CHAR8 *) malloc (1);   if (mOptions.OutputDirectory == NULL) {@@
> -679,7 +681,7 @@ CVfrCompiler::Compile (
>  DebugError (NULL, 0, 0001, "Error opening the input file", "%s",
> InFileName); goto Fail;   }-+  InputInfo.IsCatchDefaultEnable =
> mOptions.IsCatchDefaultEnable;   if (mOptions.HasOverrideClassGuid)
> { InputInfo.OverrideClassGuid =} else
> {@@ -937,5 +939,3 @@ main (
> return GetUtilityStatus (); }--diff --git 
> a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> index b6e207d2ce..974f37c4eb 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> @@ -52,6 +52,7 @@ typedef struct {
>BOOLEAN WarningAsError;   BOOLEAN AutoDefault;   BOOLEAN
> CheckDefault;+  BOOLEAN IsCatchDefaultEnable; } OPTIONS;  typedef enum 
> {diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp
> b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> index 65bb8e34fd..8a706f929b 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> @@ -49,7 +49,8 @@ static SVFR_WARNING_HANDLE VFR_WARNING_HANDLE_TABLE 
> [] = {
>{ VFR_WARNING_DEFAULT_VALUE_REDEFINED, ": default value re-defined 
> 

Re: [edk2-devel] [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

2024-03-08 Thread Zhang, Zifeng
Hi Liming,

Could you help to review the patch for VFR compiler change submitted by Yuting?

Btw, 
Dell need some introduction of Hii StructurePCD implementation especially for 
replacing patch table configurations.
Would you like to share EDK2 wiki link or doc for it?

Best Regards,
Zifeng


-Original Message-
From: Yang, Yuting2  
Sent: Friday, January 26, 2024 10:54 AM
To: devel@edk2.groups.io
Cc: Rebecca Cran ; Liming Gao ; 
Feng, Bob C ; Chen, Christine ; 
Zhang, Zifeng 
Subject: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

Add --catch_default option to raise a DefaultValueError when encountering VFR 
default definitions to help remove default variables.

Signed-off-by: Yuting Yang 

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Christine Chen 
Cc: Zifeng Zhang 
Signed-off-by: Yuting Yang 
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp |   8 +-
 BaseTools/Source/C/VfrCompile/VfrCompiler.h   |   1 +
 BaseTools/Source/C/VfrCompile/VfrError.cpp|   3 +-
 BaseTools/Source/C/VfrCompile/VfrError.h  |   3 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h|   1 +
 BaseTools/Source/C/VfrCompile/VfrSyntax.g | 238 ++
 6 files changed, 150 insertions(+), 104 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 5f4d262d85..4031af6e39 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -78,6 +78,7 @@ CVfrCompiler::OptionInitialization (
   mOptions.WarningAsError= FALSE;   mOptions.AutoDefault   
= FALSE;   mOptions.CheckDefault  = FALSE;+  
mOptions.IsCatchDefaultEnable  = FALSE;   memset 
(, 0, sizeof (EFI_GUID));if (Argc == 1) {@@ 
-95,6 +96,8 @@ CVfrCompiler::OptionInitialization (
   Version ();   SET_RUN_STATUS (STATUS_DEAD);   return;+} else 
if (stricmp(Argv[Index], "--catch_default") == 0){+  
mOptions.IsCatchDefaultEnable = TRUE; } else if (stricmp(Argv[Index], "-l") 
== 0) {   mOptions.CreateRecordListFile = TRUE;   
gCIfrRecordInfoDB.TurnOn ();@@ -179,7 +182,6 @@ 
CVfrCompiler::OptionInitialization (
   goto Fail; } strcpy (mOptions.VfrFileName, Argv[Index]);- if 
(mOptions.OutputDirectory == NULL) {   mOptions.OutputDirectory = (CHAR8 *) 
malloc (1);   if (mOptions.OutputDirectory == NULL) {@@ -679,7 +681,7 @@ 
CVfrCompiler::Compile (
 DebugError (NULL, 0, 0001, "Error opening the input file", "%s", 
InFileName); goto Fail;   }-+  InputInfo.IsCatchDefaultEnable = 
mOptions.IsCatchDefaultEnable;   if (mOptions.HasOverrideClassGuid) { 
InputInfo.OverrideClassGuid =} else {@@ -937,5 
+939,3 @@ main (
return GetUtilityStatus (); }--diff --git 
a/BaseTools/Source/C/VfrCompile/VfrCompiler.h 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
index b6e207d2ce..974f37c4eb 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
@@ -52,6 +52,7 @@ typedef struct {
   BOOLEAN WarningAsError;   BOOLEAN AutoDefault;   BOOLEAN CheckDefault;+  
BOOLEAN IsCatchDefaultEnable; } OPTIONS;  typedef enum {diff --git 
a/BaseTools/Source/C/VfrCompile/VfrError.cpp 
b/BaseTools/Source/C/VfrCompile/VfrError.cpp
index 65bb8e34fd..8a706f929b 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
@@ -49,7 +49,8 @@ static SVFR_WARNING_HANDLE VFR_WARNING_HANDLE_TABLE [] = {
   { VFR_WARNING_DEFAULT_VALUE_REDEFINED, ": default value re-defined with 
different value"},   { VFR_WARNING_ACTION_WITH_TEXT_TWO, ": Action opcode 
should not have TextTwo part"},   { VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE, ": 
Not recommend to use obsoleted framework opcode"},-  { 
VFR_WARNING_CODEUNDEFINED, ": undefined Warning Code" }+  { 
VFR_WARNING_CODEUNDEFINED, ": undefined Warning Code" },+  { 
VFR_WARNING_UNSUPPORTED, ": pls remove the default values if necessary" } };  
CVfrErrorHandle::CVfrErrorHandle (diff --git 
a/BaseTools/Source/C/VfrCompile/VfrError.h 
b/BaseTools/Source/C/VfrCompile/VfrError.h
index 7d16bd5f74..1b4bc173d2 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.h
+++ b/BaseTools/Source/C/VfrCompile/VfrError.h
@@ -47,7 +47,8 @@ typedef enum {
   VFR_WARNING_DEFAULT_VALUE_REDEFINED = 0,   VFR_WARNING_ACTION_WITH_TEXT_TWO, 
  VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE,-  VFR_WARNING_CODEUNDEFINED+  
VFR_WARNING_CODEUNDEFINED,+  VFR_WARNING_UNSUPPORTED } EFI_VFR_WARNING_CODE;  
typedef struct _SVFR_ERROR_HANDLE {diff --git 
a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h 
b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 9ef6f07787..d8fada3bcb 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -96,6 +96,7 @@ struct SBufferNode {
  typedef struct {   EFI_GUID *OverrideClassGuid;+  BOOLEAN 
IsCatchDefaultEnable; } INPUT_INFO_TO_SYNTAX;  class CFormPkg {diff --git 

[edk2-devel] [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

2024-01-25 Thread Yuting Yang
Add --catch_default option to raise a DefaultValueError when
encountering VFR default definitions to help remove default variables.

Signed-off-by: Yuting Yang 

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Christine Chen 
Cc: Zifeng Zhang 
Signed-off-by: Yuting Yang 
---
 BaseTools/Source/C/VfrCompile/VfrCompiler.cpp |   8 +-
 BaseTools/Source/C/VfrCompile/VfrCompiler.h   |   1 +
 BaseTools/Source/C/VfrCompile/VfrError.cpp|   3 +-
 BaseTools/Source/C/VfrCompile/VfrError.h  |   3 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h|   1 +
 BaseTools/Source/C/VfrCompile/VfrSyntax.g | 238 ++
 6 files changed, 150 insertions(+), 104 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
index 5f4d262d85..4031af6e39 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
@@ -78,6 +78,7 @@ CVfrCompiler::OptionInitialization (
   mOptions.WarningAsError= FALSE;
   mOptions.AutoDefault   = FALSE;
   mOptions.CheckDefault  = FALSE;
+  mOptions.IsCatchDefaultEnable  = FALSE;
   memset (, 0, sizeof (EFI_GUID));
 
   if (Argc == 1) {
@@ -95,6 +96,8 @@ CVfrCompiler::OptionInitialization (
   Version ();
   SET_RUN_STATUS (STATUS_DEAD);
   return;
+} else if (stricmp(Argv[Index], "--catch_default") == 0){
+  mOptions.IsCatchDefaultEnable = TRUE;
 } else if (stricmp(Argv[Index], "-l") == 0) {
   mOptions.CreateRecordListFile = TRUE;
   gCIfrRecordInfoDB.TurnOn ();
@@ -179,7 +182,6 @@ CVfrCompiler::OptionInitialization (
   goto Fail;
 }
 strcpy (mOptions.VfrFileName, Argv[Index]);
-
 if (mOptions.OutputDirectory == NULL) {
   mOptions.OutputDirectory = (CHAR8 *) malloc (1);
   if (mOptions.OutputDirectory == NULL) {
@@ -679,7 +681,7 @@ CVfrCompiler::Compile (
 DebugError (NULL, 0, 0001, "Error opening the input file", "%s", 
InFileName);
 goto Fail;
   }
-
+  InputInfo.IsCatchDefaultEnable = mOptions.IsCatchDefaultEnable;
   if (mOptions.HasOverrideClassGuid) {
 InputInfo.OverrideClassGuid = 
   } else {
@@ -937,5 +939,3 @@ main (
 
   return GetUtilityStatus ();
 }
-
-
diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.h 
b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
index b6e207d2ce..974f37c4eb 100644
--- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
+++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
@@ -52,6 +52,7 @@ typedef struct {
   BOOLEAN WarningAsError;
   BOOLEAN AutoDefault;
   BOOLEAN CheckDefault;
+  BOOLEAN IsCatchDefaultEnable;
 } OPTIONS;
 
 typedef enum {
diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp 
b/BaseTools/Source/C/VfrCompile/VfrError.cpp
index 65bb8e34fd..8a706f929b 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
@@ -49,7 +49,8 @@ static SVFR_WARNING_HANDLE VFR_WARNING_HANDLE_TABLE [] = {
   { VFR_WARNING_DEFAULT_VALUE_REDEFINED, ": default value re-defined with 
different value"},
   { VFR_WARNING_ACTION_WITH_TEXT_TWO, ": Action opcode should not have TextTwo 
part"},
   { VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE, ": Not recommend to use obsoleted 
framework opcode"},
-  { VFR_WARNING_CODEUNDEFINED, ": undefined Warning Code" }
+  { VFR_WARNING_CODEUNDEFINED, ": undefined Warning Code" },
+  { VFR_WARNING_UNSUPPORTED, ": pls remove the default values if necessary" }
 };
 
 CVfrErrorHandle::CVfrErrorHandle (
diff --git a/BaseTools/Source/C/VfrCompile/VfrError.h 
b/BaseTools/Source/C/VfrCompile/VfrError.h
index 7d16bd5f74..1b4bc173d2 100644
--- a/BaseTools/Source/C/VfrCompile/VfrError.h
+++ b/BaseTools/Source/C/VfrCompile/VfrError.h
@@ -47,7 +47,8 @@ typedef enum {
   VFR_WARNING_DEFAULT_VALUE_REDEFINED = 0,
   VFR_WARNING_ACTION_WITH_TEXT_TWO,
   VFR_WARNING_OBSOLETED_FRAMEWORK_OPCODE,
-  VFR_WARNING_CODEUNDEFINED
+  VFR_WARNING_CODEUNDEFINED,
+  VFR_WARNING_UNSUPPORTED
 } EFI_VFR_WARNING_CODE;
 
 typedef struct _SVFR_ERROR_HANDLE {
diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h 
b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
index 9ef6f07787..d8fada3bcb 100644
--- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
+++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.h
@@ -96,6 +96,7 @@ struct SBufferNode {
 
 typedef struct {
   EFI_GUID *OverrideClassGuid;
+  BOOLEAN IsCatchDefaultEnable;
 } INPUT_INFO_TO_SYNTAX;
 
 class CFormPkg {
diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 55fd067f8a..5daf1c423c 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -50,6 +50,7 @@ VfrParserStart (
 {
   ParserBlackBox VfrParser(File);
   VfrParser.parser()->SetOverrideClassGuid (InputInfo->OverrideClassGuid);
+  VfrParser.parser()->SetIsCatchDefaultEnable(InputInfo->IsCatchDefaultEnable);
   return VfrParser.parser()->vfrProgram();
 }
 >>
@@ -386,8 +387,8 @@