Re: [PATCH] MdeModulePkg/Terminal: Check status of OpenProtocol in BindingStart

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

Indeed UEFI Spec requires that Supported() should be called before Start().
But Steven's test cases directly call Start() which exposes this "issue".
I don't want to argue the call doesn't follow UEFI Spec.
Fixing it doesn't harm:).


> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, April 13, 2018 1:15 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Shi, Steven <steven.shi@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH] MdeModulePkg/Terminal: Check status of
> OpenProtocol in BindingStart
> 
> 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?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@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.zeng@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.ni@intel.com>
> Cc: Steven Shi <steven.shi@intel.com>
> Cc: Star Zeng <star.zeng@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