Thanks Laszlo.

If the commit log is updated to reference the BZ link or even add more detailed 
information.
Reviewed-by: Star Zeng <star.z...@intel.com>


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, April 13, 2018 5:00 PM
To: Zeng, Star <star.z...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; 
edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/Terminal: Check status of OpenProtocol 
in BindingStart

Hi Star,

On 04/13/18 07:15, Zeng, Star wrote:
> Generally, I am ok with the change.
> But, what is the case for the change?
> If the new "if condition" is TRUE, the ASSERT above it will happen, so the 
> new "if condition" is for RELEASE build case?
> And if the code passes Support() check, then seemingly, the "if condition" in 
> Start() will not happen.
> Could more detailed information in the commit log or code comments?

I believe this patch is for
<https://bugzilla.tianocore.org/show_bug.cgi?id=917>. It seems to be for the 
"input Controller handle is invalid" case.

Comment 0 in that BZ describes the issue, but I agree the commit message should 
at least reference the BZ.

Thanks
Laszlo


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Friday, April 13, 2018 12:14 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.z...@intel.com>
> Subject: [edk2] [PATCH] MdeModulePkg/Terminal: Check status of 
> OpenProtocol in BindingStart
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Steven Shi <steven....@intel.com>
> Cc: Star Zeng <star.z...@intel.com>
> ---
>  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c 
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> index 60de2d4d6d..66dd3ad550 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> @@ -2,7 +2,7 @@
>    Produces Simple Text Input Protocol, Simple Text Input Extended Protocol 
> and
>    Simple Text Output Protocol upon Serial IO Protocol.
>  
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2006 - 2018, 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  which accompanies this 
> distribution.  The full text of the license may be found at @@ -488,6 +488,9 
> @@ TerminalDriverBindingStart (
>                    EFI_OPEN_PROTOCOL_BY_DRIVER
>                    );
>    ASSERT ((Status == EFI_SUCCESS) || (Status == 
> EFI_ALREADY_STARTED));
> +  if (EFI_ERROR (Status) && (Status != EFI_ALREADY_STARTED)) {
> +    return Status;
> +  }
>  
>    //
>    // Open the Serial I/O Protocol BY_DRIVER.  It might already be started.
> @@ -501,6 +504,9 @@ TerminalDriverBindingStart (
>                    EFI_OPEN_PROTOCOL_BY_DRIVER
>                    );
>    ASSERT ((Status == EFI_SUCCESS) || (Status == 
> EFI_ALREADY_STARTED));
> +  if (EFI_ERROR (Status) && (Status != EFI_ALREADY_STARTED)) {
> +    return Status;
> +  }
>  
>    if (!IsHotPlugDevice (ParentDevicePath)) {
>      //
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to