From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by groups.io with SMTP; Wed, 08 May 2019 01:50:43 -0700
Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id DADE73097063;
	Wed,  8 May 2019 08:50:42 +0000 (UTC)
Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-255.rdu2.redhat.com [10.10.120.255])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 661505DAAF;
	Wed,  8 May 2019 08:50:40 +0000 (UTC)
Subject: Re: [PATCH V2 1/6] MdeModulePkg/ResetSystemLib.h: Add useful reset data definition
To: Zhichao Gao <zhichao.gao@intel.com>, devel@edk2.groups.io
Cc: Jian J Wang <jian.j.wang@intel.com>, Hao Wu <hao.a.wu@intel.com>,
 Ray Ni <ray.ni@intel.com>, Star Zeng <star.zeng@intel.com>,
 Liming Gao <liming.gao@intel.com>, Sean Brogan <sean.brogan@microsoft.com>,
 Michael Turner <Michael.Turner@microsoft.com>,
 Bret Barkelew <Bret.Barkelew@microsoft.com>
References: <20190508044939.15744-1-zhichao.gao@intel.com>
 <20190508044939.15744-2-zhichao.gao@intel.com>
From: "Laszlo Ersek" <lersek@redhat.com>
Message-ID: <bfb8e6bf-a891-4a78-77ff-88e5e8679402@redhat.com>
Date: Wed, 8 May 2019 10:50:39 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20190508044939.15744-2-zhichao.gao@intel.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Wed, 08 May 2019 08:50:43 +0000 (UTC)
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit

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