From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id B8EE021184E9D for ; Thu, 1 Nov 2018 13:23:28 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DCBD6A78; Thu, 1 Nov 2018 13:23:27 -0700 (PDT) Received: from supven01-VirtualBox (U203142.usa.Arm.com [10.118.30.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id ABE3D3F71D; Thu, 1 Nov 2018 13:23:27 -0700 (PDT) Message-ID: From: Supreeth Venkatesh To: Eric Jin , edk2-devel@lists.01.org Date: Thu, 01 Nov 2018 15:23:25 -0500 In-Reply-To: <20181101025259.14644-1-eric.jin@intel.com> References: <20181101025259.14644-1-eric.jin@intel.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 Mime-Version: 1.0 Subject: Re: [edk2-test][Patch v2] uefi-sct/SctPkg:Assign 0 to the tail of HwErrRecVariableName. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Nov 2018 20:23:28 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Reviewed-by: Supreeth Venkatesh There are some unintentional indentation changes in "switch" statement. Please take care of that before (if intentional, ok as well) commit. On Thu, 2018-11-01 at 10:52 +0800, Eric Jin wrote: > Add definition of HwErrRecVariableNamePrefixLength, > HwErrRecVariableNameIndexLength and HwErrRecVariableNameLength > Make the HwErrRecVariableName as the valid string. > Ensure the HwErrRecVariable could be deleted before the test exit. > > Cc: Supreeth Venkatesh > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Jin > --- > .../BlackBoxTest/VariableServicesBBTestFunction.c | 38 > +++++++++++++--------- > .../BlackBoxTest/VariableServicesBBTestMain.h | 13 +++++++- > 2 files changed, 35 insertions(+), 16 deletions(-) > > 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 d1064ce..defe71a 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 > @@ -1,7 +1,7 @@ > /** @file > > Copyright 2006 - 2012 Unified EFI, Inc.
> - Copyright (c) 2010 - 2012, Intel Corporation. All rights > reserved.
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights > reserved.
> > This program and the accompanying materials > are licensed and made available under the terms and conditions of > the BSD License > @@ -2855,7 +2855,7 @@ HardwareErrorRecordFuncTest ( > UINT64 RemainingVariableStorageSize; > UINT64 MaximumVariableSize; > > - CHAR16 HwErrRecVariableName[13]; > + CHAR16 HwErrRecVariableName[HwErrRecVariableNameLen > gth]; > 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[5]; > + CHAR16 ErrorNum[HwErrRecVariableNameIndexLength+1]; > > CHAR16 HwErrRecGetVariable[255]; > > @@ -2908,7 +2908,11 @@ HardwareErrorRecordFuncTest ( > } > > // > - // Read reset record > + // Try to read reset record from the RecoveryData, > + // and the magic num is saved in the RecoveryData[0]. > + // When the status is EFI_SUCCESS and magic num is 2, > + // it means useful data has been saved before the reset > + // and the date should be retrived goto particular process > // > Status = RecoveryLib->ReadResetRecord ( > RecoveryLib, > @@ -2916,12 +2920,12 @@ HardwareErrorRecordFuncTest ( > RecoveryData > ); > if ( !EFI_ERROR(Status) && (RecoveryDataSize > 0) ) { > - switch (RecoveryData[0]) { > + switch (RecoveryData[0]) { Is the indentation change intentional? It looked good to me earlier and is ok with edk2 C coding standards as well. > case 2: > goto step2; > default: > goto step3; > - } > + } Is the indentation change intentional? It looked good to me earlier and is ok with edk2 C coding standards as well. > } > > // > @@ -2978,7 +2982,7 @@ HardwareErrorRecordFuncTest ( > // Get a useable variable name > // > GetVariableName[0] = L'\0'; > - ErrorNum[4] = L'\0'; > + ErrorNum[HwErrRecVariableNameIndexLength] = L'\0'; > > > while (TRUE) { > @@ -3001,9 +3005,9 @@ HardwareErrorRecordFuncTest ( > break; > } > > - if ( (SctStrnCmp (GetVariableName, L"HwErrRec", 8) == 0) && > + if ( (SctStrnCmp (GetVariableName, L"HwErrRec", > HwErrRecVariableNamePrefixLength) == 0) && > (SctCompareGuid (&VendorGuid, &gHwErrRecGuid) == 0) ) { > - SctStrnCpy (ErrorNum, &GetVariableName[8], 4); > + SctStrnCpy (ErrorNum, > &GetVariableName[HwErrRecVariableNamePrefixLength], > HwErrRecVariableNameIndexLength); > Num = SctXtoi (ErrorNum); > if (MaxNum < Num) > MaxNum = Num; > @@ -3014,7 +3018,8 @@ HardwareErrorRecordFuncTest ( > > HwErrRecVariableName[0] = L'\0'; > SctStrCat ( HwErrRecVariableName, L"HwErrRec" ); > - Myitox( MaxNum, HwErrRecVariableName+8 ); > + Myitox( MaxNum, > HwErrRecVariableName+HwErrRecVariableNamePrefixLength ); > + HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0'; > > // > // Set the new HwErrRec variable to the global variable > @@ -3033,11 +3038,12 @@ HardwareErrorRecordFuncTest ( > } > > // > - // Write reset record > + // Before the reset, test writes magic num 2 in RecoveryData[0] > + // and writes the useful data - HwErrRecVariableName - to > RecoveryData[2] > // > RecoveryData[0] = 2; > - SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, 12 > ); > - RecoveryLib->WriteResetRecord( RecoveryLib, 13*sizeof(CHAR16)+2, > RecoveryData ); > + SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, > HwErrRecVariableNameLength-1 ); > + RecoveryLib->WriteResetRecord( RecoveryLib, > HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData ); > > // > // Prompt the user about the cold reset and reset the system > @@ -3048,11 +3054,13 @@ HardwareErrorRecordFuncTest ( > gtRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL); > > // > - // After the cold reset > + // The particular process after the reset > + // retrive the useful data - HwErrRecVariableName - from > RecoveryData[2] > // > step2: > DataSize = 255; > - SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), 12 > ); > + HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0'; > + SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), > HwErrRecVariableNameLength-1 ); > Status = RT->GetVariable ( > HwErrRecVariableName, > &gHwErrRecGuid, > 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 051ae6f..426b762 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 > @@ -1,7 +1,7 @@ > /** @file > > Copyright 2006 - 2016 Unified EFI, Inc.
> - Copyright (c) 2010 - 2016, Intel Corporation. All rights > reserved.
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights > reserved.
> > This program and the accompanying materials > are licensed and made available under the terms and conditions of > the BSD License > @@ -125,6 +125,17 @@ Abstract: > #endif > > // > +// The Variable Name of Hardware Error Record Variables > +// defined in the UEFI Spec is HwErrRec####. For example, > +// HwErrRec0001, HwErrRec0002, HwErrRecF31A, etc. > +// 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 > + > +// > // Global Variables > // >