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

["Daryl McDaniel \(EDK2 Lists\)" <edk2-lists@mc2research.org>]

Thomas,

I'll take a look at this.
Have you determined what the impact of changing the signature of the functions (adding/removing parameters) will be?  Can it
adversely affect existing code?

Sincerely,
Daryl McDaniel


> -----Original Message-----
> From: Palmer, Thomas [mailto:thomas.palmer@hpe.com]
> Sent: Monday, September 11, 2017 12:35 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; edk2-
> lists@mc2research.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
> 
> 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