Re: [PATCH V2 1/6] MdeModulePkg/ResetSystemLib.h: Add useful reset data definition

["Laszlo Ersek" <lersek@redhat.com>]

On 05/08/19 06:49, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772
> 
> Add a useful definition of reset data which start with a NULL
> string and followed by a EFI_GUID.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/Include/Library/ResetSystemLib.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Library/ResetSystemLib.h b/MdeModulePkg/Include/Library/ResetSystemLib.h
> index 3b2a098a60..e39e8c748c 100644
> --- a/MdeModulePkg/Include/Library/ResetSystemLib.h
> +++ b/MdeModulePkg/Include/Library/ResetSystemLib.h
> @@ -13,6 +13,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Uefi/UefiBaseType.h>
>  #include <Uefi/UefiMultiPhase.h>
>  
> +#pragma pack(1)
> +typedef struct {
> +  CHAR16      NullString;
> +  EFI_GUID    ResetGuid;
> +} RESET_DATA_WITH_NULL_STRING;
> +#pragma pack()
> +
> +VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);
> +
>  /**
>    This function causes a system-wide reset (cold reset), in which
>    all circuitry within the system returns to its initial state. This type of reset
> 

(1) Please insert a space character between "pack" and "(". (Two
instances in the patch.)

(2) Personally I think the structure is small and clear enough for it to
need no comments. The MdeModulePkg maintainers might still request some
light comments just above the structure.

With (1) updated, and regardless of (2):

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo