[edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test

[edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test

[Eric Jin]

Name macros appropriately to follow the rule
in coding standards specification.
Change the following macro from variable style
HwErrRecVariableNameLength
HwErrRecVariableNamePrefixLength
HwErrRecVariableNameIndexLength
to macro style.
HW_ERR_REC_VARIABLE_NAME_LEN
HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN
HW_ERR_REC_VARIABLE_NAME_INDEX_LEN

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin <eric.jin@intel.com>
---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h     |  6 +++---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c | 22 +++++++++++-----------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
index 426b762..7eaa56d 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
@@ -131,9 +131,9 @@ Abstract:
 // The prefix length is 8, index length is 4.
 // Consider the tail of string, the name length is 13.
 //
-#define HwErrRecVariableNameLength       13
-#define HwErrRecVariableNamePrefixLength 8
-#define HwErrRecVariableNameIndexLength  4
+#define HW_ERR_REC_VARIABLE_NAME_LEN           13
+#define HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN    8
+#define HW_ERR_REC_VARIABLE_NAME_INDEX_LEN     4
 
 //
 // Global Variables
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
index a016476..015a78a 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
@@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest (
   UINT64                RemainingVariableStorageSize;
   UINT64                MaximumVariableSize;
   
-  CHAR16                HwErrRecVariableName[HwErrRecVariableNameLength];
+  CHAR16                HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN];
   CHAR16                HwErrRecVariable[] = L"This is a HwErrRec variable!";
   
   CHAR16                GetVariableName[MAX_BUFFER_SIZE];
@@ -2864,7 +2864,7 @@ HardwareErrorRecordFuncTest (
   
   UINTN                 Num;
   UINTN                 MaxNum = 0;
-  CHAR16                ErrorNum[HwErrRecVariableNameIndexLength+1];
+  CHAR16                ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN + 1];
   
   CHAR16                HwErrRecGetVariable[255];
   
@@ -2982,7 +2982,7 @@ HardwareErrorRecordFuncTest (
   // Get a useable variable name
   //
   GetVariableName[0] = L'\0';
-  ErrorNum[HwErrRecVariableNameIndexLength] = L'\0';
+  ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN] = L'\0';
   
 
   while (TRUE) {
@@ -3005,9 +3005,9 @@ HardwareErrorRecordFuncTest (
       break;
     }
 
-    if ( (SctStrnCmp (GetVariableName, L"HwErrRec", HwErrRecVariableNamePrefixLength) == 0) &&
+    if ( (SctStrnCmp (GetVariableName, L"HwErrRec", HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN) == 0) &&
          (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) {
-      SctStrnCpy (ErrorNum, &GetVariableName[HwErrRecVariableNamePrefixLength], HwErrRecVariableNameIndexLength);
+      SctStrnCpy (ErrorNum, &GetVariableName[HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN], HW_ERR_REC_VARIABLE_NAME_INDEX_LEN);
       Num = SctXtoi (ErrorNum);
       if (MaxNum < Num)
         MaxNum = Num;
@@ -3018,8 +3018,8 @@ HardwareErrorRecordFuncTest (
     
   HwErrRecVariableName[0] = L'\0';
   SctStrCat ( HwErrRecVariableName, L"HwErrRec" );
-  Myitox( MaxNum, HwErrRecVariableName+HwErrRecVariableNamePrefixLength );
-  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
+  Myitox( MaxNum, HwErrRecVariableName + HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN );
+  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
   
   //
   // Set the new HwErrRec variable to the global variable
@@ -3042,8 +3042,8 @@ HardwareErrorRecordFuncTest (
   // and writes the useful data - HwErrRecVariableName - to RecoveryData[2]
   //
   RecoveryData[0] = 2;
-  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, HwErrRecVariableNameLength-1 );
-  RecoveryLib->WriteResetRecord( RecoveryLib, HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData );
+  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
+  RecoveryLib->WriteResetRecord( RecoveryLib, HW_ERR_REC_VARIABLE_NAME_LEN * sizeof(CHAR16) + 2, RecoveryData );
   
   //
   // Prompt the user about the cold reset and reset the system
@@ -3059,8 +3059,8 @@ HardwareErrorRecordFuncTest (
   //
 step2:
   DataSize = 255;
-  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
-  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), HwErrRecVariableNameLength-1 );
+  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
+  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
   Status = RT->GetVariable (
                         HwErrRecVariableName,
                         &gHwErrRecGuid,
-- 
2.20.0.windows.1

Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test

[Leif Lindholm]

Hi Eric,

On Wed, Dec 12, 2018 at 11:32:14AM +0800, Eric Jin wrote:
> Name macros appropriately to follow the rule
> in coding standards specification.
> Change the following macro from variable style
> HwErrRecVariableNameLength
> HwErrRecVariableNamePrefixLength
> HwErrRecVariableNameIndexLength
> to macro style.
> HW_ERR_REC_VARIABLE_NAME_LEN
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN
> HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
>  uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h     |  6 +++---
>  uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c | 22 +++++++++++-----------
>  2 files changed, 14 insertions(+), 14 deletions(-)

Both the patch generation and the code changes look good to me,
thanks!
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> 
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
> index 426b762..7eaa56d 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestMain.h
> @@ -131,9 +131,9 @@ Abstract:
>  // The prefix length is 8, index length is 4.
>  // Consider the tail of string, the name length is 13.
>  //
> -#define HwErrRecVariableNameLength       13
> -#define HwErrRecVariableNamePrefixLength 8
> -#define HwErrRecVariableNameIndexLength  4
> +#define HW_ERR_REC_VARIABLE_NAME_LEN           13
> +#define HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN    8
> +#define HW_ERR_REC_VARIABLE_NAME_INDEX_LEN     4
>  
>  //
>  // Global Variables
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
> index a016476..015a78a 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestFunction.c
> @@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest (
>    UINT64                RemainingVariableStorageSize;
>    UINT64                MaximumVariableSize;
>    
> -  CHAR16                HwErrRecVariableName[HwErrRecVariableNameLength];
> +  CHAR16                HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN];
>    CHAR16                HwErrRecVariable[] = L"This is a HwErrRec variable!";
>    
>    CHAR16                GetVariableName[MAX_BUFFER_SIZE];
> @@ -2864,7 +2864,7 @@ HardwareErrorRecordFuncTest (
>    
>    UINTN                 Num;
>    UINTN                 MaxNum = 0;
> -  CHAR16                ErrorNum[HwErrRecVariableNameIndexLength+1];
> +  CHAR16                ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN + 1];
>    
>    CHAR16                HwErrRecGetVariable[255];
>    
> @@ -2982,7 +2982,7 @@ HardwareErrorRecordFuncTest (
>    // Get a useable variable name
>    //
>    GetVariableName[0] = L'\0';
> -  ErrorNum[HwErrRecVariableNameIndexLength] = L'\0';
> +  ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN] = L'\0';
>    
>  
>    while (TRUE) {
> @@ -3005,9 +3005,9 @@ HardwareErrorRecordFuncTest (
>        break;
>      }
>  
> -    if ( (SctStrnCmp (GetVariableName, L"HwErrRec", HwErrRecVariableNamePrefixLength) == 0) &&
> +    if ( (SctStrnCmp (GetVariableName, L"HwErrRec", HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN) == 0) &&
>           (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) {
> -      SctStrnCpy (ErrorNum, &GetVariableName[HwErrRecVariableNamePrefixLength], HwErrRecVariableNameIndexLength);
> +      SctStrnCpy (ErrorNum, &GetVariableName[HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN], HW_ERR_REC_VARIABLE_NAME_INDEX_LEN);
>        Num = SctXtoi (ErrorNum);
>        if (MaxNum < Num)
>          MaxNum = Num;
> @@ -3018,8 +3018,8 @@ HardwareErrorRecordFuncTest (
>      
>    HwErrRecVariableName[0] = L'\0';
>    SctStrCat ( HwErrRecVariableName, L"HwErrRec" );
> -  Myitox( MaxNum, HwErrRecVariableName+HwErrRecVariableNamePrefixLength );
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> +  Myitox( MaxNum, HwErrRecVariableName + HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
>    
>    //
>    // Set the new HwErrRec variable to the global variable
> @@ -3042,8 +3042,8 @@ HardwareErrorRecordFuncTest (
>    // and writes the useful data - HwErrRecVariableName - to RecoveryData[2]
>    //
>    RecoveryData[0] = 2;
> -  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, HwErrRecVariableNameLength-1 );
> -  RecoveryLib->WriteResetRecord( RecoveryLib, HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData );
> +  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
> +  RecoveryLib->WriteResetRecord( RecoveryLib, HW_ERR_REC_VARIABLE_NAME_LEN * sizeof(CHAR16) + 2, RecoveryData );
>    
>    //
>    // Prompt the user about the cold reset and reset the system
> @@ -3059,8 +3059,8 @@ HardwareErrorRecordFuncTest (
>    //
>  step2:
>    DataSize = 255;
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> -  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), HwErrRecVariableNameLength-1 );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
> +  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
>    Status = RT->GetVariable (
>                          HwErrRecVariableName,
>                          &gHwErrRecGuid,
> -- 
> 2.20.0.windows.1
> 

Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test

[Supreeth Venkatesh]

Eric,

Nothing wrong with code.

However, when applying this patch with git am, I encounter the below
errors. (not sure if it is related to mailbox configuration).
Not sure if it is my mailbox, could you please test it on your side
using git am and let me know?

git am
./patches/0001_SctPkg\:Correct_macro_name_style_in_HwErrRecVariable_Te
st.mbox
Applying: uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable
Test
error: patch failed: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestMain.h:131
error: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestMain.h: patch does not apply
error: patch failed: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestFunction.c:2855
error: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestFunction.c: patch does not apply
Patch failed at 0001 uefi-sct/SctPkg:Correct macro name style in
HwErrRecVariable Test
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Thanks,
Supreeth
On Wed, 2018-12-12 at 11:32 +0800, Eric Jin wrote:
> Name macros appropriately to follow the rule
> in coding standards specification.
> Change the following macro from variable style
> HwErrRecVariableNameLength
> HwErrRecVariableNamePrefixLength
> HwErrRecVariableNameIndexLength
> to macro style.
> HW_ERR_REC_VARIABLE_NAME_LEN
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN
> HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
>  uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h     |  6 +++---
>  uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c | 22 +++++++++++-----------
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> index 426b762..7eaa56d 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> @@ -131,9 +131,9 @@ Abstract:
>  // The prefix length is 8, index length is 4.
>  // Consider the tail of string, the name length is 13.
>  //
> -#define HwErrRecVariableNameLength       13
> -#define HwErrRecVariableNamePrefixLength 8
> -#define HwErrRecVariableNameIndexLength  4
> +#define HW_ERR_REC_VARIABLE_NAME_LEN           13
> +#define HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN    8
> +#define HW_ERR_REC_VARIABLE_NAME_INDEX_LEN     4
>  
>  //
>  // Global Variables
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> index a016476..015a78a 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> @@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest (
>    UINT64                RemainingVariableStorageSize;
>    UINT64                MaximumVariableSize;
>    
> -  CHAR16                HwErrRecVariableName[HwErrRecVariableNameLen
> gth];
> +  CHAR16                HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAM
> E_LEN];
>    CHAR16                HwErrRecVariable[] = L"This is a HwErrRec
> variable!";
>    
>    CHAR16                GetVariableName[MAX_BUFFER_SIZE];
> @@ -2864,7 +2864,7 @@ HardwareErrorRecordFuncTest (
>    
>    UINTN                 Num;
>    UINTN                 MaxNum = 0;
> -  CHAR16                ErrorNum[HwErrRecVariableNameIndexLength+1];
> +  CHAR16                ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> + 1];
>    
>    CHAR16                HwErrRecGetVariable[255];
>    
> @@ -2982,7 +2982,7 @@ HardwareErrorRecordFuncTest (
>    // Get a useable variable name
>    //
>    GetVariableName[0] = L'\0';
> -  ErrorNum[HwErrRecVariableNameIndexLength] = L'\0';
> +  ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN] = L'\0';
>    
>  
>    while (TRUE) {
> @@ -3005,9 +3005,9 @@ HardwareErrorRecordFuncTest (
>        break;
>      }
>  
> -    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> HwErrRecVariableNamePrefixLength) == 0) &&
> +    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN) == 0) &&
>           (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) {
> -      SctStrnCpy (ErrorNum,
> &GetVariableName[HwErrRecVariableNamePrefixLength],
> HwErrRecVariableNameIndexLength);
> +      SctStrnCpy (ErrorNum,
> &GetVariableName[HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN],
> HW_ERR_REC_VARIABLE_NAME_INDEX_LEN);
>        Num = SctXtoi (ErrorNum);
>        if (MaxNum < Num)
>          MaxNum = Num;
> @@ -3018,8 +3018,8 @@ HardwareErrorRecordFuncTest (
>      
>    HwErrRecVariableName[0] = L'\0';
>    SctStrCat ( HwErrRecVariableName, L"HwErrRec" );
> -  Myitox( MaxNum,
> HwErrRecVariableName+HwErrRecVariableNamePrefixLength );
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> +  Myitox( MaxNum, HwErrRecVariableName +
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
>    
>    //
>    // Set the new HwErrRec variable to the global variable
> @@ -3042,8 +3042,8 @@ HardwareErrorRecordFuncTest (
>    // and writes the useful data - HwErrRecVariableName - to
> RecoveryData[2]
>    //
>    RecoveryData[0] = 2;
> -  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> HwErrRecVariableNameLength-1 );
> -  RecoveryLib->WriteResetRecord( RecoveryLib,
> HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData );
> +  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
> +  RecoveryLib->WriteResetRecord( RecoveryLib,
> HW_ERR_REC_VARIABLE_NAME_LEN * sizeof(CHAR16) + 2, RecoveryData );
>    
>    //
>    // Prompt the user about the cold reset and reset the system
> @@ -3059,8 +3059,8 @@ HardwareErrorRecordFuncTest (
>    //
>  step2:
>    DataSize = 255;
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> -  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> HwErrRecVariableNameLength-1 );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
> +  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
>    Status = RT->GetVariable (
>                          HwErrRecVariableName,
>                          &gHwErrRecGuid,

Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test

[Jin, Eric]

Hello Supreeth,

I use "Apply Patch Serial" operation provided by TortoiseGit to do the applying.
The operation is equivalent to "git.exe am --3way --ignore-space-change --keep-cr Fix.patch"

What is your command to apply patch?

I observe the same failure with "git.exe am Fix.patch" and "--ignore-space-change " is the key.
I am not sure if it is the mail-patch-conversion cause or not.

And the patch "[edk2][edk2-test][PATCH v1 1/1] uefi-sct: Change line endings to CR LF. "  also has the same failure behavior without "--ignore-space-change " on my side. 

Best Regards
Eric

-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com> 
Sent: Thursday, December 13, 2018 5:07 AM
To: Jin, Eric <eric.jin@intel.com>; edk2-devel@lists.01.org
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test


Eric,

Nothing wrong with code.

However, when applying this patch with git am, I encounter the below errors. (not sure if it is related to mailbox configuration).
Not sure if it is my mailbox, could you please test it on your side using git am and let me know?

git am
./patches/0001_SctPkg\:Correct_macro_name_style_in_HwErrRecVariable_Te
st.mbox
Applying: uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test
error: patch failed: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestMain.h:131
error: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestMain.h: patch does not apply
error: patch failed: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestFunction.c:2855
error: uefi-
sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
est/VariableServicesBBTestFunction.c: patch does not apply Patch failed at 0001 uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Thanks,
Supreeth
On Wed, 2018-12-12 at 11:32 +0800, Eric Jin wrote:
> Name macros appropriately to follow the rule in coding standards 
> specification.
> Change the following macro from variable style 
> HwErrRecVariableNameLength HwErrRecVariableNamePrefixLength 
> HwErrRecVariableNameIndexLength to macro style.
> HW_ERR_REC_VARIABLE_NAME_LEN
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN
> HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
>  uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h     |  6 +++---
>  uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c | 22 +++++++++++-----------
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h b/uefi- 
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> index 426b762..7eaa56d 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestMain.h
> @@ -131,9 +131,9 @@ Abstract:
>  // The prefix length is 8, index length is 4.
>  // Consider the tail of string, the name length is 13.
>  //
> -#define HwErrRecVariableNameLength       13
> -#define HwErrRecVariableNamePrefixLength 8 -#define 
> HwErrRecVariableNameIndexLength  4
> +#define HW_ERR_REC_VARIABLE_NAME_LEN           13
> +#define HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN    8
> +#define HW_ERR_REC_VARIABLE_NAME_INDEX_LEN     4
>  
>  //
>  // Global Variables
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c b/uefi- 
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> index a016476..015a78a 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> xTest/VariableServicesBBTestFunction.c
> @@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest (
>    UINT64                RemainingVariableStorageSize;
>    UINT64                MaximumVariableSize;
>    
> -  CHAR16                HwErrRecVariableName[HwErrRecVariableNameLen
> gth];
> +  CHAR16                HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAM
> E_LEN];
>    CHAR16                HwErrRecVariable[] = L"This is a HwErrRec
> variable!";
>    
>    CHAR16                GetVariableName[MAX_BUFFER_SIZE];
> @@ -2864,7 +2864,7 @@ HardwareErrorRecordFuncTest (
>    
>    UINTN                 Num;
>    UINTN                 MaxNum = 0;
> -  CHAR16                ErrorNum[HwErrRecVariableNameIndexLength+1];
> +  CHAR16                ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> + 1];
>    
>    CHAR16                HwErrRecGetVariable[255];
>    
> @@ -2982,7 +2982,7 @@ HardwareErrorRecordFuncTest (
>    // Get a useable variable name
>    //
>    GetVariableName[0] = L'\0';
> -  ErrorNum[HwErrRecVariableNameIndexLength] = L'\0';
> +  ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN] = L'\0';
>    
>  
>    while (TRUE) {
> @@ -3005,9 +3005,9 @@ HardwareErrorRecordFuncTest (
>        break;
>      }
>  
> -    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> HwErrRecVariableNamePrefixLength) == 0) &&
> +    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN) == 0) &&
>           (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) {
> -      SctStrnCpy (ErrorNum,
> &GetVariableName[HwErrRecVariableNamePrefixLength],
> HwErrRecVariableNameIndexLength);
> +      SctStrnCpy (ErrorNum,
> &GetVariableName[HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN],
> HW_ERR_REC_VARIABLE_NAME_INDEX_LEN);
>        Num = SctXtoi (ErrorNum);
>        if (MaxNum < Num)
>          MaxNum = Num;
> @@ -3018,8 +3018,8 @@ HardwareErrorRecordFuncTest (
>      
>    HwErrRecVariableName[0] = L'\0';
>    SctStrCat ( HwErrRecVariableName, L"HwErrRec" );
> -  Myitox( MaxNum,
> HwErrRecVariableName+HwErrRecVariableNamePrefixLength );
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> +  Myitox( MaxNum, HwErrRecVariableName +
> HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
>    
>    //
>    // Set the new HwErrRec variable to the global variable @@ -3042,8 
> +3042,8 @@ HardwareErrorRecordFuncTest (
>    // and writes the useful data - HwErrRecVariableName - to 
> RecoveryData[2]
>    //
>    RecoveryData[0] = 2;
> -  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> HwErrRecVariableNameLength-1 );
> -  RecoveryLib->WriteResetRecord( RecoveryLib, 
> HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData );
> +  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
> +  RecoveryLib->WriteResetRecord( RecoveryLib,
> HW_ERR_REC_VARIABLE_NAME_LEN * sizeof(CHAR16) + 2, RecoveryData );
>    
>    //
>    // Prompt the user about the cold reset and reset the system @@ 
> -3059,8 +3059,8 @@ HardwareErrorRecordFuncTest (
>    //
>  step2:
>    DataSize = 255;
> -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> -  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> HwErrRecVariableNameLength-1 );
> +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';  
> + SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
>    Status = RT->GetVariable (
>                          HwErrRecVariableName,
>                          &gHwErrRecGuid,

Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Leif Lindholm]

Hmm, this gets me thinking...

We were discussing before about doing a line ending conversion in
edk2, and let the git gools provide native line endings (as designed).

Is this a good opportunity to run a pilot with edk2-test, where much
less history will be lost?

Regards,

Leif

On Fri, Dec 14, 2018 at 07:12:31AM +0000, Jin, Eric wrote:
> Hello Supreeth,
> 
> I use "Apply Patch Serial" operation provided by TortoiseGit to do the applying.
> The operation is equivalent to "git.exe am --3way --ignore-space-change --keep-cr Fix.patch"
> 
> What is your command to apply patch?
> 
> I observe the same failure with "git.exe am Fix.patch" and "--ignore-space-change " is the key.
> I am not sure if it is the mail-patch-conversion cause or not.
> 
> And the patch "[edk2][edk2-test][PATCH v1 1/1] uefi-sct: Change line endings to CR LF. "  also has the same failure behavior without "--ignore-space-change " on my side. 
> 
> Best Regards
> Eric
> 
> -----Original Message-----
> From: Supreeth Venkatesh <supreeth.venkatesh@arm.com> 
> Sent: Thursday, December 13, 2018 5:07 AM
> To: Jin, Eric <eric.jin@intel.com>; edk2-devel@lists.01.org
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test
> 
> 
> Eric,
> 
> Nothing wrong with code.
> 
> However, when applying this patch with git am, I encounter the below errors. (not sure if it is related to mailbox configuration).
> Not sure if it is my mailbox, could you please test it on your side using git am and let me know?
> 
> git am
> ./patches/0001_SctPkg\:Correct_macro_name_style_in_HwErrRecVariable_Te
> st.mbox
> Applying: uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test
> error: patch failed: uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
> est/VariableServicesBBTestMain.h:131
> error: uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
> est/VariableServicesBBTestMain.h: patch does not apply
> error: patch failed: uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
> est/VariableServicesBBTestFunction.c:2855
> error: uefi-
> sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxT
> est/VariableServicesBBTestFunction.c: patch does not apply Patch failed at 0001 uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Thanks,
> Supreeth
> On Wed, 2018-12-12 at 11:32 +0800, Eric Jin wrote:
> > Name macros appropriately to follow the rule in coding standards 
> > specification.
> > Change the following macro from variable style 
> > HwErrRecVariableNameLength HwErrRecVariableNamePrefixLength 
> > HwErrRecVariableNameIndexLength to macro style.
> > HW_ERR_REC_VARIABLE_NAME_LEN
> > HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN
> > HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> > 
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Jin <eric.jin@intel.com>
> > ---
> >  uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestMain.h     |  6 +++---
> >  uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestFunction.c | 22 +++++++++++-----------
> >  2 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestMain.h b/uefi- 
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestMain.h
> > index 426b762..7eaa56d 100644
> > --- a/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestMain.h
> > +++ b/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestMain.h
> > @@ -131,9 +131,9 @@ Abstract:
> >  // The prefix length is 8, index length is 4.
> >  // Consider the tail of string, the name length is 13.
> >  //
> > -#define HwErrRecVariableNameLength       13
> > -#define HwErrRecVariableNamePrefixLength 8 -#define 
> > HwErrRecVariableNameIndexLength  4
> > +#define HW_ERR_REC_VARIABLE_NAME_LEN           13
> > +#define HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN    8
> > +#define HW_ERR_REC_VARIABLE_NAME_INDEX_LEN     4
> >  
> >  //
> >  // Global Variables
> > diff --git a/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestFunction.c b/uefi- 
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestFunction.c
> > index a016476..015a78a 100644
> > --- a/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestFunction.c
> > +++ b/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBo
> > xTest/VariableServicesBBTestFunction.c
> > @@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest (
> >    UINT64                RemainingVariableStorageSize;
> >    UINT64                MaximumVariableSize;
> >    
> > -  CHAR16                HwErrRecVariableName[HwErrRecVariableNameLen
> > gth];
> > +  CHAR16                HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAM
> > E_LEN];
> >    CHAR16                HwErrRecVariable[] = L"This is a HwErrRec
> > variable!";
> >    
> >    CHAR16                GetVariableName[MAX_BUFFER_SIZE];
> > @@ -2864,7 +2864,7 @@ HardwareErrorRecordFuncTest (
> >    
> >    UINTN                 Num;
> >    UINTN                 MaxNum = 0;
> > -  CHAR16                ErrorNum[HwErrRecVariableNameIndexLength+1];
> > +  CHAR16                ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN
> > + 1];
> >    
> >    CHAR16                HwErrRecGetVariable[255];
> >    
> > @@ -2982,7 +2982,7 @@ HardwareErrorRecordFuncTest (
> >    // Get a useable variable name
> >    //
> >    GetVariableName[0] = L'\0';
> > -  ErrorNum[HwErrRecVariableNameIndexLength] = L'\0';
> > +  ErrorNum[HW_ERR_REC_VARIABLE_NAME_INDEX_LEN] = L'\0';
> >    
> >  
> >    while (TRUE) {
> > @@ -3005,9 +3005,9 @@ HardwareErrorRecordFuncTest (
> >        break;
> >      }
> >  
> > -    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> > HwErrRecVariableNamePrefixLength) == 0) &&
> > +    if ( (SctStrnCmp (GetVariableName, L"HwErrRec",
> > HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN) == 0) &&
> >           (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) {
> > -      SctStrnCpy (ErrorNum,
> > &GetVariableName[HwErrRecVariableNamePrefixLength],
> > HwErrRecVariableNameIndexLength);
> > +      SctStrnCpy (ErrorNum,
> > &GetVariableName[HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN],
> > HW_ERR_REC_VARIABLE_NAME_INDEX_LEN);
> >        Num = SctXtoi (ErrorNum);
> >        if (MaxNum < Num)
> >          MaxNum = Num;
> > @@ -3018,8 +3018,8 @@ HardwareErrorRecordFuncTest (
> >      
> >    HwErrRecVariableName[0] = L'\0';
> >    SctStrCat ( HwErrRecVariableName, L"HwErrRec" );
> > -  Myitox( MaxNum,
> > HwErrRecVariableName+HwErrRecVariableNamePrefixLength );
> > -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> > +  Myitox( MaxNum, HwErrRecVariableName +
> > HW_ERR_REC_VARIABLE_NAME_PREFIX_LEN );
> > +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';
> >    
> >    //
> >    // Set the new HwErrRec variable to the global variable @@ -3042,8 
> > +3042,8 @@ HardwareErrorRecordFuncTest (
> >    // and writes the useful data - HwErrRecVariableName - to 
> > RecoveryData[2]
> >    //
> >    RecoveryData[0] = 2;
> > -  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> > HwErrRecVariableNameLength-1 );
> > -  RecoveryLib->WriteResetRecord( RecoveryLib, 
> > HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData );
> > +  SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName,
> > HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
> > +  RecoveryLib->WriteResetRecord( RecoveryLib,
> > HW_ERR_REC_VARIABLE_NAME_LEN * sizeof(CHAR16) + 2, RecoveryData );
> >    
> >    //
> >    // Prompt the user about the cold reset and reset the system @@ 
> > -3059,8 +3059,8 @@ HardwareErrorRecordFuncTest (
> >    //
> >  step2:
> >    DataSize = 255;
> > -  HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0';
> > -  SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> > HwErrRecVariableNameLength-1 );
> > +  HwErrRecVariableName[HW_ERR_REC_VARIABLE_NAME_LEN - 1] = L'\0';  
> > + SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2),
> > HW_ERR_REC_VARIABLE_NAME_LEN - 1 );
> >    Status = RT->GetVariable (
> >                          HwErrRecVariableName,
> >                          &gHwErrRecGuid,
> 

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Laszlo Ersek]

On 12/14/18 11:59, Leif Lindholm wrote:
> Hmm, this gets me thinking...
> 
> We were discussing before about doing a line ending conversion in
> edk2, and let the git gools provide native line endings (as designed).
> 
> Is this a good opportunity to run a pilot with edk2-test, where much
> less history will be lost?

Well, history won't be lost, in the sense that people running "git
blame" will need one more execution of "git blame" (to "look past" the
whitespace change commit), but yes, it will result in a minor inconvenience.

And, I think, converting the edk2-test repo would not be a bad test at all.

Thanks,
Laszlo

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Supreeth Venkatesh]

On Fri, 2018-12-14 at 14:24 +0100, Laszlo Ersek wrote:
> On 12/14/18 11:59, Leif Lindholm wrote:
> > Hmm, this gets me thinking...
> > 
> > We were discussing before about doing a line ending conversion in
> > edk2, and let the git gools provide native line endings (as
> > designed).
> > 
> > Is this a good opportunity to run a pilot with edk2-test, where
> > much
> > less history will be lost?
> 
> Well, history won't be lost, in the sense that people running "git
> blame" will need one more execution of "git blame" (to "look past"
> the
> whitespace change commit), but yes, it will result in a minor
> inconvenience.
> 
> And, I think, converting the edk2-test repo would not be a bad test
> at all.
> 
Thanks Leif/Laszlo. I volunteer to try the tool. However, I admit that
I have not tried this before, any pointers/instructions on how to do
this?

> Thanks,
> Laszlo

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Laszlo Ersek]

On 12/14/18 18:12, Supreeth Venkatesh wrote:
> On Fri, 2018-12-14 at 14:24 +0100, Laszlo Ersek wrote:
>> On 12/14/18 11:59, Leif Lindholm wrote:
>>> Hmm, this gets me thinking...
>>>
>>> We were discussing before about doing a line ending conversion in
>>> edk2, and let the git gools provide native line endings (as
>>> designed).
>>>
>>> Is this a good opportunity to run a pilot with edk2-test, where
>>> much
>>> less history will be lost?
>>
>> Well, history won't be lost, in the sense that people running "git
>> blame" will need one more execution of "git blame" (to "look past"
>> the
>> whitespace change commit), but yes, it will result in a minor
>> inconvenience.
>>
>> And, I think, converting the edk2-test repo would not be a bad test
>> at all.
>>
> Thanks Leif/Laszlo. I volunteer to try the tool. However, I admit that
> I have not tried this before, any pointers/instructions on how to do
> this?

I imagine you'd run a "find" command to locate all source/text files
(skip ".git"), then feed them to xargs / dos2unix.

The trick is more in the git settings, once the internal representation
has been converted to LF only. I'm thinking that "core.whitespace",
"am.keepcr" and "core.autocrlf" should be set the "right way". (= to be
researched)

Thanks
Laszlo

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Supreeth Venkatesh]

On Fri, 2018-12-14 at 20:57 +0100, Laszlo Ersek wrote:
> On 12/14/18 18:12, Supreeth Venkatesh wrote:
> > On Fri, 2018-12-14 at 14:24 +0100, Laszlo Ersek wrote:
> > > On 12/14/18 11:59, Leif Lindholm wrote:
> > > > Hmm, this gets me thinking...
> > > > 
> > > > We were discussing before about doing a line ending conversion
> > > > in
> > > > edk2, and let the git gools provide native line endings (as
> > > > designed).
> > > > 
> > > > Is this a good opportunity to run a pilot with edk2-test, where
> > > > much
> > > > less history will be lost?
> > > 
> > > Well, history won't be lost, in the sense that people running
> > > "git
> > > blame" will need one more execution of "git blame" (to "look
> > > past"
> > > the
> > > whitespace change commit), but yes, it will result in a minor
> > > inconvenience.
> > > 
> > > And, I think, converting the edk2-test repo would not be a bad
> > > test
> > > at all.
> > > 
> > 
> > Thanks Leif/Laszlo. I volunteer to try the tool. However, I admit
> > that
> > I have not tried this before, any pointers/instructions on how to
> > do
> > this?
> 
> I imagine you'd run a "find" command to locate all source/text files
> (skip ".git"), then feed them to xargs / dos2unix.
> 
> The trick is more in the git settings, once the internal
> representation
> has been converted to LF only. I'm thinking that "core.whitespace",
> "am.keepcr" and "core.autocrlf" should be set the "right way". (= to
> be
> researched)
Never mind. I think I misread the initial email. I was expecting some
git tool magic or script :). I can come up with one. Thanks.

> 
> Thanks
> Laszlo

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Jin, Eric]

Currently, the git am fail with " --ignore-space-change" switch on/off. It is better if we can solve it with "core.whitespace", "am.keepcr" and "core.autocrlf" in the meantime.

Should all existing patches hold until the conversion done or the conversion is tried on the branch until it is validated? 

Best Regards
Eric

-----Original Message-----
From: Supreeth Venkatesh [mailto:supreeth.venkatesh@arm.com] 
Sent: Saturday, December 15, 2018 7:54 AM
To: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: edk2-devel@lists.01.org; Jin, Eric <eric.jin@intel.com>
Subject: Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

On Fri, 2018-12-14 at 20:57 +0100, Laszlo Ersek wrote:
> On 12/14/18 18:12, Supreeth Venkatesh wrote:
> > On Fri, 2018-12-14 at 14:24 +0100, Laszlo Ersek wrote:
> > > On 12/14/18 11:59, Leif Lindholm wrote:
> > > > Hmm, this gets me thinking...
> > > > 
> > > > We were discussing before about doing a line ending conversion 
> > > > in edk2, and let the git gools provide native line endings (as 
> > > > designed).
> > > > 
> > > > Is this a good opportunity to run a pilot with edk2-test, where 
> > > > much less history will be lost?
> > > 
> > > Well, history won't be lost, in the sense that people running "git 
> > > blame" will need one more execution of "git blame" (to "look past"
> > > the
> > > whitespace change commit), but yes, it will result in a minor 
> > > inconvenience.
> > > 
> > > And, I think, converting the edk2-test repo would not be a bad 
> > > test at all.
> > > 
> > 
> > Thanks Leif/Laszlo. I volunteer to try the tool. However, I admit
> > that
> > I have not tried this before, any pointers/instructions on how to
> > do
> > this?
> 
> I imagine you'd run a "find" command to locate all source/text files
> (skip ".git"), then feed them to xargs / dos2unix.
> 
> The trick is more in the git settings, once the internal
> representation
> has been converted to LF only. I'm thinking that "core.whitespace",
> "am.keepcr" and "core.autocrlf" should be set the "right way". (= to
> be
> researched)
Never mind. I think I misread the initial email. I was expecting some
git tool magic or script :). I can come up with one. Thanks.

> 
> Thanks
> Laszlo

Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"

[Laszlo Ersek]

On 12/15/18 04:59, Jin, Eric wrote:
> Currently, the git am fail with " --ignore-space-change" switch on/off. It is better if we can solve it with "core.whitespace", "am.keepcr" and "core.autocrlf" in the meantime.
> 
> Should all existing patches hold until the conversion done or the conversion is tried on the branch until it is validated? 

If you just want to apply a patch from the mailing list, while sticking
with the current line endings (=CRLF), then please use the git settings
described e.g. at:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05


Note that applying an edk2[-*] patch from the mailing list with git-am
might fail for some reasons even if your git settings are fine (as
described above).

(1) One is that the submitter might have messed up the patch, and added
LF-terminated lines to a CRLF context.

(2) Another is that, for a good while, "git-am" would reject
"/dev/null\r\n" header lines -- /dev/null lines are used when a patch
adds or removes a file --, and only accept "/dev/null\n" ones. This is a
problem because "am.keepcr" is required for edk2 patches (for now), and
that results in \r *not* being stripped from "/dev/null\r\n" either.

In other words, without am.keepcr, the patch context wouldn't match, and
with am.keepcr, git would reject /dev/null lines (i.e., patches that
added or removed files).


In order to remedy (2), I had submitted a trivial (one-liner) patch for
the git project in the year 2014:

https://public-inbox.org/git/1411434583-27692-1-git-send-email-lersek@redhat.com/

The patch was rejected. Thus I've had to carry it in my own personal git
build, until quite recently, in order to more easily work with edk2 patches.

Thankfully, the issue was finally fixed in the following two git.git
commits:

* 8bc172e5f298 ("apply: use starts_with() in gitdiff_verify_name()",
2017-07-01)
* e454ad4becff ("apply: handle Subversion diffs with /dev/null
gracefully", 2018-02-15)

(In terms of git releases, this means v2.17.0.)

... If you asked me why my patch wasn't originally accepted, but was
accepted from another contributor, approx. 3.5+ years later (such that
the new commit message would even reference "git-for-windows", i.e. it'd
target a use case that was *very similar* to mine), then my answer would
be: because that's how open source works; your success depends on what
mood you catch a maintainer in.

--------

Anyway, regarding the conversion, for the long term:

- All text files should first be converted to LF only. (With a series of
commits.)

- For people on Windows, they should set "core.autocrlf" in their local
git configs.

- "am.keepcr" should be cleared by everyone, in their respective git
configs.

- "core.whitespace" might need more tweaking, but it's less important.

Thanks
Laszlo