Re: [PATCH 1/3] MdePkg: break #defines out of Uefi/UefiMultiPhase.h

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 12/13/2017 8:26 PM, Leif Lindholm wrote:
> Turns out all .vfr files in the tree interacting with DynamicPcds
> manually copy the same set of EFI_VARIABLE_* definitions, since the rest
> of UefiMultiPhase.h is incompatible with VfrCompile.
> 
> Split these out into a separate header file UefiMultiPhaseDefinitions.h
> in order to make it possible to include just that portion into .vfr
> files. Then include that from UefiMultiPhase.h.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>   MdePkg/Include/Uefi/UefiMultiPhase.h            | 23 +-----------
>   MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 ++++++++++++++++++++
>   2 files changed, 41 insertions(+), 21 deletions(-)
> 
> diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h b/MdePkg/Include/Uefi/UefiMultiPhase.h
> index 0dcbb1b9ee..b360c9513b 100644
> --- a/MdePkg/Include/Uefi/UefiMultiPhase.h
> +++ b/MdePkg/Include/Uefi/UefiMultiPhase.h
> @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>   #ifndef __UEFI_MULTIPHASE_H__
>   #define __UEFI_MULTIPHASE_H__
>   
> +#include "UefiMultiPhaseDefinitions.h"
> +
>   #include <Guid/WinCertificate.h>
>   ///
>   /// Enumeration of memory types introduced in UEFI.
> @@ -156,27 +158,6 @@ typedef struct {
>   } EFI_TABLE_HEADER;
>   
>   ///
> -/// Attributes of variable.
> -///
> -#define EFI_VARIABLE_NON_VOLATILE                            0x00000001
> -#define EFI_VARIABLE_BOOTSERVICE_ACCESS                      0x00000002
> -#define EFI_VARIABLE_RUNTIME_ACCESS                          0x00000004
> -///
> -/// This attribute is identified by the mnemonic 'HR'
> -/// elsewhere in this specification.
> -///
> -#define EFI_VARIABLE_HARDWARE_ERROR_RECORD                   0x00000008
> -///
> -/// Attributes of Authenticated Variable
> -///
> -#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x00000020
> -#define EFI_VARIABLE_APPEND_WRITE                            0x00000040
> -///
> -/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved.
> -///
> -#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS              0x00000010
> -
> -///
>   /// AuthInfo is a WIN_CERTIFICATE using the wCertificateType
>   /// WIN_CERTIFICATE_UEFI_GUID and the CertType
>   /// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies
> diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
> new file mode 100644
> index 0000000000..df55a92dfa
> --- /dev/null
> +++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
> @@ -0,0 +1,39 @@
> +/** @file
> +  This includes some definitions introduced in UEFI that will be used in both PEI and DXE phases.
> +
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials are licensed and made available under
> +the terms and conditions of the BSD License that accompanies this distribution.
> +The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php.
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __UEFI_MULTIPHASE_DEFS_H__
> +#define __UEFI_MULTIPHASE_DEFS_H__
> +
> +///
> +/// Attributes of variable.
> +///
> +#define EFI_VARIABLE_NON_VOLATILE                            0x00000001
> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS                      0x00000002
> +#define EFI_VARIABLE_RUNTIME_ACCESS                          0x00000004
> +///
> +/// This attribute is identified by the mnemonic 'HR'
> +/// elsewhere in this specification.
> +///
> +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD                   0x00000008
> +///
> +/// Attributes of Authenticated Variable
> +///
> +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS   0x00000020
> +#define EFI_VARIABLE_APPEND_WRITE                            0x00000040
> +///
> +/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved.
> +///
> +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS              0x00000010
> +
> +#endif
> 
Can we just move the definitions to UefiBaseTypes.h?
Even vfrcompiler still complains, the syntax enhancement to
vfrcompiler should be simple.

I personally do not like creating more and more files.

-- 
Thanks,
Ray