* [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions
@ 2017-08-10 22:34 Thomas Palmer
2017-08-10 22:59 ` Carsey, Jaben
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Palmer @ 2017-08-10 22:34 UTC (permalink / raw)
To: edk2-devel; +Cc: edk2-lists, jaben.carsey, joseph.shifflett, Thomas Palmer
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions
2017-08-10 22:34 [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions Thomas Palmer
@ 2017-08-10 22:59 ` Carsey, Jaben
2017-09-11 19:35 ` Palmer, Thomas
0 siblings, 1 reply; 5+ messages in thread
From: Carsey, Jaben @ 2017-08-10 22:59 UTC (permalink / raw)
To: Thomas Palmer, edk2-devel@lists.01.org
Cc: edk2-lists@mc2research.org, joseph.shifflett@hpe.com
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions
2017-08-10 22:59 ` Carsey, Jaben
@ 2017-09-11 19:35 ` Palmer, Thomas
2017-09-11 20:39 ` Daryl McDaniel (EDK2 Lists)
0 siblings, 1 reply; 5+ messages in thread
From: Palmer, Thomas @ 2017-09-11 19:35 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org,
edk2-lists@mc2research.org
Cc: edk2-lists@mc2research.org, Shifflett, Joseph
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions
2017-09-11 19:35 ` Palmer, Thomas
@ 2017-09-11 20:39 ` Daryl McDaniel (EDK2 Lists)
2017-09-11 20:53 ` Palmer, Thomas
0 siblings, 1 reply; 5+ messages in thread
From: Daryl McDaniel (EDK2 Lists) @ 2017-09-11 20:39 UTC (permalink / raw)
To: 'Palmer, Thomas', 'Carsey, Jaben', edk2-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions
2017-09-11 20:39 ` Daryl McDaniel (EDK2 Lists)
@ 2017-09-11 20:53 ` Palmer, Thomas
0 siblings, 0 replies; 5+ messages in thread
From: Palmer, Thomas @ 2017-09-11 20:53 UTC (permalink / raw)
To: Daryl McDaniel (EDK2 Lists), 'Carsey, Jaben',
edk2-devel@lists.01.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-11 20:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-10 22:34 [PATCH 1/1] StdLib/EfiSocketLib: Fix ABI mismatch for 2 event functions Thomas Palmer
2017-08-10 22:59 ` Carsey, Jaben
2017-09-11 19:35 ` Palmer, Thomas
2017-09-11 20:39 ` Daryl McDaniel (EDK2 Lists)
2017-09-11 20:53 ` Palmer, Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox