Re: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

["Palmer, Thomas" <thomas.palmer@hpe.com>]

Daryl? 

	I haven't seen a comment on this patch since 8/10


Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it shorter" - Blaise Pascal


-----Original Message-----
From: Carsey, Jaben [mailto:jaben.carsey@intel.com] 
Sent: Thursday, August 10, 2017 5:59 PM
To: Palmer, Thomas <thomas.palmer@hpe.com>; edk2-devel@lists.01.org
Cc: edk2-lists@mc2research.org; Shifflett, Joseph <joseph.shifflett@hpe.com>
Subject: RE: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

Looks good to me.

Daryl?

> -----Original Message-----
> From: Thomas Palmer [mailto:thomas.palmer@hpe.com]
> Sent: Thursday, August 10, 2017 3:35 PM
> To: edk2-devel@lists.01.org
> Cc: edk2-lists@mc2research.org; Carsey, Jaben 
> <jaben.carsey@intel.com>; joseph.shifflett@hpe.com; Thomas Palmer 
> <thomas.palmer@hpe.com>
> Subject: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event 
> functions
> Importance: High
> 
> The gBS->CreateEvent expects a EFI_EVENT_NOTIFY function as the third 
> argument. The EFIAPI token is an important component of that 
> prototype. Its absence can cause unexpected issues on DEBUG systems 
> built with GCC due to ABI mismatches.
> 
> Both EslTcp4ConnectComplete and EslTcp6ConnectComplete did not have 
> the EFIAPI token required of a EFI_EVENT_NOTIFY function. GCC did not 
> catch this because of the explicit EFI_EVENT_NOTIFY cast.  By removing 
> the cast, a build error ensues.
> 
> This patch removes the cast and updates both functions to comply with 
> EFI_EVENT_NOTIFY.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>
> ---
>  StdLib/EfiSocketLib/Tcp4.c | 8 ++++++--  StdLib/EfiSocketLib/Tcp6.c | 
> 8 ++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c 
> index 68477fba6e70..8125a8d4f5ad 100644
> --- a/StdLib/EfiSocketLib/Tcp4.c
> +++ b/StdLib/EfiSocketLib/Tcp4.c
> @@ -2,6 +2,7 @@
>    Implement the TCP4 driver support for the socket layer.
> 
>    Copyright (c) 2011 - 2015, Intel Corporation. All rights 
> reserved.<BR>
> +  (C) Copyright 2017 Hewlett Packard Enterprise Development LP<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 @@ 
> -192,9 +193,10 @@ EslTcp4Accept (
> 
>  **/
>  VOID
> +EFIAPI
>  EslTcp4ConnectComplete (
>    IN EFI_EVENT Event,
> -  IN ESL_PORT * pPort
> +  IN VOID      *Context
>    )
>  {
>    BOOLEAN bRemoveFirstPort;
> @@ -203,12 +205,14 @@ EslTcp4ConnectComplete (
>    ESL_SOCKET * pSocket;
>    ESL_TCP4_CONTEXT * pTcp4;
>    EFI_STATUS Status;
> +  ESL_PORT * pPort;
> 
>    DBG_ENTER ( );
> 
>    //
>    //  Locate the TCP context
>    //
> +  pPort = Context;
>    pSocket = pPort->pSocket;
>    pTcp4 = &pPort->Context.Tcp4;
> 
> @@ -1288,7 +1292,7 @@ EslTcp4PortAllocate (
>      //
>      Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
>                                   TPL_SOCKETS,
> -                                 (EFI_EVENT_NOTIFY)EslTcp4ConnectComplete,
> +                                 EslTcp4ConnectComplete,
>                                   pPort,
>                                   &pTcp4->ConnectToken.CompletionToken.Event);
>      if ( EFI_ERROR ( Status )) {
> diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c 
> index 0f6d2d6ac93c..9f9c00f6dc57 100644
> --- a/StdLib/EfiSocketLib/Tcp6.c
> +++ b/StdLib/EfiSocketLib/Tcp6.c
> @@ -2,6 +2,7 @@
>    Implement the TCP6 driver support for the socket layer.
> 
>    Copyright (c) 2011 - 2014, Intel Corporation. All rights 
> reserved.<BR>
> +  (C) Copyright 2017 Hewlett Packard Enterprise Development LP<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 @@ 
> -186,9 +187,10 @@ EslTcp6Accept (
> 
>  **/
>  VOID
> +EFIAPI
>  EslTcp6ConnectComplete (
>    IN EFI_EVENT Event,
> -  IN ESL_PORT * pPort
> +  IN VOID      *Context
>    )
>  {
>    BOOLEAN bRemoveFirstPort;
> @@ -197,12 +199,14 @@ EslTcp6ConnectComplete (
>    ESL_SOCKET * pSocket;
>    ESL_TCP6_CONTEXT * pTcp6;
>    EFI_STATUS Status;
> +  ESL_PORT * pPort;
> 
>    DBG_ENTER ( );
> 
>    //
>    //  Locate the TCP context
>    //
> +  pPort = Context;
>    pSocket = pPort->pSocket;
>    pTcp6 = &pPort->Context.Tcp6;
> 
> @@ -1339,7 +1343,7 @@ EslTcp6PortAllocate (
>      //
>      Status = gBS->CreateEvent (  EVT_NOTIFY_SIGNAL,
>                                   TPL_SOCKETS,
> -                                 (EFI_EVENT_NOTIFY)EslTcp6ConnectComplete,
> +                                 EslTcp6ConnectComplete,
>                                   pPort,
>                                   &pTcp6->ConnectToken.CompletionToken.Event);
>      if ( EFI_ERROR ( Status )) {
> --
> 2.7.4