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 6056221184AB3 for ; Tue, 30 Oct 2018 09:59:51 -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 A99F080D; Tue, 30 Oct 2018 09:59:50 -0700 (PDT) Received: from supven01-VirtualBox (unknown [10.119.48.94]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 2F1D73F557; Tue, 30 Oct 2018 09:59:50 -0700 (PDT) Message-ID: <7350d7ed7b696e08585dceeb88d5f379620184ee.camel@arm.com> From: Supreeth Venkatesh To: Eric Jin , edk2-devel@lists.01.org Cc: Jiaxin Wu , supreeth.venkatesh@arm.com Date: Tue, 30 Oct 2018 11:59:46 -0500 In-Reply-To: <20181030083836.7184-1-eric.jin@intel.com> References: <20181030083836.7184-1-eric.jin@intel.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 Mime-Version: 1.0 Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg:Assign 0 to the tail of the 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: Tue, 30 Oct 2018 16:59:52 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Reviewed-by: Supreeth Venkatesh If the below magic number comments(inline) are fixed before commit. On Tue, 2018-10-30 at 16:38 +0800, Eric Jin wrote: > Make the HwErrRecVariableName as valid the string. > Ensure the HwErrRecVariable could be deleted before the test exit. > > Cc: Supreeth Venkatesh > Cc: Jiaxin Wu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Jin > --- > .../BlackBoxTest/VariableServicesBBTestFunction.c | 12 > +++++++----- > .../BlackBoxTest/VariableServicesBBTestMain.h | 10 > +++++++++- > 2 files changed, 16 insertions(+), 6 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..df1bbe7 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]; > @@ -3015,6 +3015,7 @@ HardwareErrorRecordFuncTest ( > HwErrRecVariableName[0] = L'\0'; > SctStrCat ( HwErrRecVariableName, L"HwErrRec" ); > Myitox( MaxNum, HwErrRecVariableName+8 ); I understand this line is not part of this patch, but however can we define "#define" for magic number 8, while we are touching this file. > + HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0'; > > // > // Set the new HwErrRec variable to the global variable > @@ -3036,8 +3037,8 @@ HardwareErrorRecordFuncTest ( > // Write reset record > // > RecoveryData[0] = 2; I understand this line is not part of this patch, but however can we define "#define" for magic number 2, while we are touching this file. > - SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, 12 > ); > - RecoveryLib->WriteResetRecord( RecoveryLib, 13*sizeof(CHAR16)+2, > RecoveryData ); > + SctStrnCpy ( (CHAR16*)(&RecoveryData[2]), HwErrRecVariableName, > HwErrRecVariableNameLength-1 ); "#define" for magic number 2 > + RecoveryLib->WriteResetRecord( RecoveryLib, > HwErrRecVariableNameLength*sizeof(CHAR16)+2, RecoveryData ); "#define" for magic number 2 > > // > // Prompt the user about the cold reset and reset the system > @@ -3052,7 +3053,8 @@ HardwareErrorRecordFuncTest ( > // > step2: > DataSize = 255; > - SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), 12 > ); > + HwErrRecVariableName[HwErrRecVariableNameLength-1] = L'\0'; > + SctStrnCpy ( HwErrRecVariableName, (CHAR16*)(RecoveryData+2), "#define" for magic number 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..b645b55 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,14 @@ Abstract: > #endif > > // > +// The Variable Name of Hardware Error Record Variables > +// defined in the UEFI Spec is HwErrRec####. For example, > +// HwErrRec0001, HwErrRec0002, HwErrRecF31A, etc. > +// Consider the tail of string, the length is 13. > +// Good documentation. > +#define HwErrRecVariableNameLength 13 > + > +// > // Global Variables > // >