diff mbox series

[edk2,1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

Message ID 1502404487-12865-1-git-send-email-thomas.palmer@hpe.com
State New
Headers show
Series [edk2,1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions | expand

Commit Message

Palmer, Thomas Aug. 10, 2017, 10:34 p.m. UTC
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(-)

-- 
2.7.4

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

Comments

Carsey, Jaben Aug. 10, 2017, 10:59 p.m. UTC | #1
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


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Palmer, Thomas Sept. 11, 2017, 7:35 p.m. UTC | #2
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


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Daryl McDaniel \(EDK2 Lists\) Sept. 11, 2017, 8:39 p.m. UTC | #3
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


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Palmer, Thomas Sept. 11, 2017, 8:53 p.m. UTC | #4
Hi Daryl,

	The gist of the patch is to have these callbacks match the EFI_EVENT_NOTIFY when compiled in GCC/DEBUG.   The EFIAPI token's absence is not noticed in Visual Studio builds b/c VS builds everything with Microsoft's ABI.   However, with GCC/DEBUG the EFIAPI token is critical for instructing compiler which ABI to use.   (I believe GCC/RELEASE uses Microsoft ABI).

	The other piece of the patch is simply changing the callback's second argument type to match EFI_EVENT_NOTIFY. I created a new variable in each function with the callback's old argument type and name so the rest of the function can remain unchanged.

	You should not notice any functional differences for Visual Studio and GCC RELEASE builds.  However, GCC DEBUG builds should now function correctly.

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: Daryl McDaniel (EDK2 Lists) [mailto:edk2-lists@mc2research.org] 

Sent: Monday, September 11, 2017 3:40 PM
To: Palmer, Thomas <thomas.palmer@hpe.com>; 'Carsey, Jaben' <jaben.carsey@intel.com>; edk2-devel@lists.01.org
Cc: Shifflett, Joseph <joseph.shifflett@hpe.com>
Subject: RE: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions

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


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

Patch

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 )) {