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:[email protected]]
Sent: Monday, September 11, 2017 3:40 PM
To: Palmer, Thomas <[email protected]>; 'Carsey, Jaben'
<[email protected]>; [email protected]
Cc: Shifflett, Joseph <[email protected]>
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:[email protected]]
> Sent: Monday, September 11, 2017 12:35 PM
> To: Carsey, Jaben <[email protected]>; [email protected];
> edk2- [email protected]
> Cc: [email protected]; Shifflett, Joseph
> <[email protected]>
> 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:[email protected]]
> Sent: Thursday, August 10, 2017 5:59 PM
> To: Palmer, Thomas <[email protected]>; [email protected]
> Cc: [email protected]; Shifflett, Joseph
> <[email protected]>
> 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:[email protected]]
> > Sent: Thursday, August 10, 2017 3:35 PM
> > To: [email protected]
> > Cc: [email protected]; Carsey, Jaben
> > <[email protected]>; [email protected]; Thomas Palmer
> > <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
