| Message ID | 1414576838-30522-1-git-send-email-olivier.martin@arm.com |
|---|---|
| State | New |
| Headers | show |
I forgot to add to the commit message but some of the fix should prevent
some errors too. Example:
Status = pSocketProtocol->pfnListen ( pSocketProtocol,
backlog,
&errno );
+ if (EFI_ERROR (Status)) {
+ errno = -1;
+ }
Before my changes we relied on pSocketProtocol->pfnListen() to always set
errno which might not be the case.
> -----Original Message-----
> From: Scott Duplichan [mailto:scott@notabs.org]
> Sent: 29 October 2014 14:29
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [PATCH] StdLib/LibC: Removed/Fixed unused variable
> to build with GCC
>
> Olivier Martin [mailto:olivier.martin@arm.com] wrote:
>
> ]Some of the variables are unused that trigger a GCC warning/error.
> ]
> ]Contributed-under: TianoCore Contribution Agreement 1.0
> ]Signed-off-by: Olivier Martin <olivier.martin@arm.com>
>
>
> Thanks Oliver. I ran this through some Visual Studio build
> testing and it is OK there.
>
> Thanks,
> Scott
>
>
> ]---
> StdLib/BsdSocketLib/bind.c | 3 +++
> StdLib/BsdSocketLib/getnameinfo.c | 8 ++++++--
> StdLib/BsdSocketLib/getsockopt.c | 3 +++
> StdLib/BsdSocketLib/listen.c | 3 +++
> StdLib/BsdSocketLib/poll.c | 3 +++
> StdLib/BsdSocketLib/res_comp.c | 4 ++--
> StdLib/BsdSocketLib/res_mkupdate.c | 3 +--
> StdLib/BsdSocketLib/res_update.c | 4 ----
> StdLib/BsdSocketLib/setsockopt.c | 3 +++
> StdLib/EfiSocketLib/Ip4.c | 12 ------------
> StdLib/EfiSocketLib/Socket.c | 1 +
> StdLib/EfiSocketLib/Tcp4.c | 4 ----
> StdLib/EfiSocketLib/Tcp6.c | 4 ----
> StdLib/LibC/Containers/Queues/Fifo.c | 2 --
> StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c | 10 ++++++----
> StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c | 3 ---
> StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c | 3 +--
> StdLib/LibC/Uefi/writev.c | 4 ++--
> 18 files changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/StdLib/BsdSocketLib/bind.c b/StdLib/BsdSocketLib/bind.c
> index f544c94..f22fe2f 100644
> --- a/StdLib/BsdSocketLib/bind.c
> +++ b/StdLib/BsdSocketLib/bind.c
> @@ -65,6 +65,9 @@ bind (
> name,
> namelen,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
>
> //
> diff --git a/StdLib/BsdSocketLib/getnameinfo.c
> b/StdLib/BsdSocketLib/getnameinfo.c
> index fab3460..0318d93 100644
> --- a/StdLib/BsdSocketLib/getnameinfo.c
> +++ b/StdLib/BsdSocketLib/getnameinfo.c
> @@ -411,15 +411,19 @@ ip6_sa2str(
> int flags
> )
> {
> - unsigned int ifindex;
> +#if 0
> + unsigned int ifindex;
> const struct in6_addr *a6;
> +#endif
> int n;
>
> _DIAGASSERT(sa6 != NULL);
> _DIAGASSERT(buf != NULL);
>
> - ifindex = (unsigned int)sa6->sin6_scope_id;
> +#if 0
> + ifindex = (unsigned int)sa6->sin6_scope_id;
> a6 = &sa6->sin6_addr;
> +#endif
>
> #ifdef NI_NUMERICSCOPE
> if ((flags & NI_NUMERICSCOPE) != 0) {
> diff --git a/StdLib/BsdSocketLib/getsockopt.c
> b/StdLib/BsdSocketLib/getsockopt.c
> index 47b7c6f..fdbd6f7 100644
> --- a/StdLib/BsdSocketLib/getsockopt.c
> +++ b/StdLib/BsdSocketLib/getsockopt.c
> @@ -60,6 +60,9 @@ getsockopt (
> option_value,
> option_len,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
>
> //
> diff --git a/StdLib/BsdSocketLib/listen.c
> b/StdLib/BsdSocketLib/listen.c
> index 7c6d5f3..d3060a4 100644
> --- a/StdLib/BsdSocketLib/listen.c
> +++ b/StdLib/BsdSocketLib/listen.c
> @@ -58,6 +58,9 @@ listen (
> Status = pSocketProtocol->pfnListen ( pSocketProtocol,
> backlog,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
>
> //
> diff --git a/StdLib/BsdSocketLib/poll.c b/StdLib/BsdSocketLib/poll.c
> index dc17567..dcc367a 100644
> --- a/StdLib/BsdSocketLib/poll.c
> +++ b/StdLib/BsdSocketLib/poll.c
> @@ -49,6 +49,9 @@ BslSocketPoll (
> Events,
> &DetectedEvents,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
>
> //
> diff --git a/StdLib/BsdSocketLib/res_comp.c
> b/StdLib/BsdSocketLib/res_comp.c
> index cddda3e..7b5f2aa 100644
> --- a/StdLib/BsdSocketLib/res_comp.c
> +++ b/StdLib/BsdSocketLib/res_comp.c
> @@ -168,7 +168,7 @@ res_hnok(
> const char *dn
> )
> {
> - int ppch = '\0', pch = PERIOD, ch = *dn++;
> + int pch = PERIOD, ch = *dn++;
>
> while (ch != '\0') {
> int nch = *dn++;
> @@ -185,7 +185,7 @@ res_hnok(
> if (!middlechar(ch))
> return (0);
> }
> - ppch = pch, pch = ch, ch = nch;
> + pch = ch, ch = nch;
> }
> return (1);
> }
> diff --git a/StdLib/BsdSocketLib/res_mkupdate.c
> b/StdLib/BsdSocketLib/res_mkupdate.c
> index 3201f31..1056fc6 100644
> --- a/StdLib/BsdSocketLib/res_mkupdate.c
> +++ b/StdLib/BsdSocketLib/res_mkupdate.c
> @@ -100,7 +100,7 @@ int
> res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) {
> ns_updrec *rrecp_start = rrecp_in;
> HEADER *hp;
> - u_char *cp, *sp1, *sp2, *startp, *endp;
> + u_char *cp, *sp2, *startp, *endp;
> int n, i, soanum, multiline;
> ns_updrec *rrecp;
> struct in_addr ina;
> @@ -125,7 +125,6 @@ res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int
> buflen) {
> hp->id = htons(++_res.id);
> hp->opcode = ns_o_update;
> hp->rcode = NOERROR;
> - sp1 = buf + 2*INT16SZ; /* save pointer to zocount */
> cp = buf + HFIXEDSZ;
> buflen -= HFIXEDSZ;
> dpp = dnptrs;
> diff --git a/StdLib/BsdSocketLib/res_update.c
> b/StdLib/BsdSocketLib/res_update.c
> index a01d832..f3ff0ce 100644
> --- a/StdLib/BsdSocketLib/res_update.c
> +++ b/StdLib/BsdSocketLib/res_update.c
> @@ -120,7 +120,6 @@ res_update(ns_updrec *rrecp_in) {
> int i, j, k = 0, n, ancount, nscount, arcount, rcode, rdatasize,
> newgroup, done, myzone, seen_before, numzones = 0;
> u_int16_t dlen, class, qclass, type, qtype;
> - u_int32_t ttl;
>
> if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> @@ -302,7 +301,6 @@ res_update(ns_updrec *rrecp_in) {
> if (cp + INT32SZ + INT16SZ > eom)
> return (-1);
> /* continue processing the soa record */
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> @@ -424,7 +422,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n",
> return (-1);
> GETSHORT(type, cp);
> GETSHORT(class, cp);
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> @@ -450,7 +447,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n",
> return (-1);
> GETSHORT(type, cp);
> GETSHORT(class, cp);
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> diff --git a/StdLib/BsdSocketLib/setsockopt.c
> b/StdLib/BsdSocketLib/setsockopt.c
> index 64f3a35..7db7dea 100644
> --- a/StdLib/BsdSocketLib/setsockopt.c
> +++ b/StdLib/BsdSocketLib/setsockopt.c
> @@ -59,6 +59,9 @@ setsockopt (
> option_value,
> option_len,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
>
> //
> diff --git a/StdLib/EfiSocketLib/Ip4.c b/StdLib/EfiSocketLib/Ip4.c
> index ed71194..46425db 100644
> --- a/StdLib/EfiSocketLib/Ip4.c
> +++ b/StdLib/EfiSocketLib/Ip4.c
> @@ -242,8 +242,6 @@ EslIp4OptionSet (
> )
> {
> BOOLEAN bTrueFalse;
> - socklen_t LengthInBytes;
> - UINT8 * pOptionData;
> EFI_STATUS Status;
>
> DBG_ENTER ( );
> @@ -257,8 +255,6 @@ EslIp4OptionSet (
> //
> // Determine if the option protocol matches
> //
> - LengthInBytes = 0;
> - pOptionData = NULL;
> switch ( OptionName ) {
> default:
> //
> @@ -283,12 +279,6 @@ EslIp4OptionSet (
> bTrueFalse = FALSE;
> }
> pOptionValue = &bTrueFalse;
> -
> - //
> - // Set the option value
> - //
> - pOptionData = (UINT8 *)&pSocket->bIncludeHeader;
> - LengthInBytes = sizeof ( pSocket->bIncludeHeader );
> }
> break;
> }
> @@ -653,7 +643,6 @@ EslIp4RxComplete (
> )
> {
> size_t LengthInBytes;
> - ESL_PORT * pPort;
> ESL_PACKET * pPacket;
> EFI_IP4_RECEIVE_DATA * pRxData;
> EFI_STATUS Status;
> @@ -663,7 +652,6 @@ EslIp4RxComplete (
> //
> // Get the operation status.
> //
> - pPort = pIo->pPort;
> Status = pIo->Token.Ip4Rx.Status;
>
> //
> diff --git a/StdLib/EfiSocketLib/Socket.c
> b/StdLib/EfiSocketLib/Socket.c
> index d53473e..740602d 100644
> --- a/StdLib/EfiSocketLib/Socket.c
> +++ b/StdLib/EfiSocketLib/Socket.c
> @@ -4130,6 +4130,7 @@ EslSocketPortCloseComplete (
> //
> Status = EslSocketPortCloseRxDone ( pPort );
> DBG_EXIT_STATUS ( Status );
> + ASSERT_EFI_ERROR (Status);
> }
>
>
> diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c
> index 7ece38d..0419ee2 100644
> --- a/StdLib/EfiSocketLib/Tcp4.c
> +++ b/StdLib/EfiSocketLib/Tcp4.c
> @@ -840,7 +840,6 @@ EslTcp4ListenComplete (
> EFI_HANDLE ChildHandle;
> struct sockaddr_in LocalAddress;
> EFI_TCP4_CONFIG_DATA * pConfigData;
> - ESL_LAYER * pLayer;
> ESL_PORT * pNewPort;
> ESL_SOCKET * pNewSocket;
> ESL_SOCKET * pSocket;
> @@ -869,7 +868,6 @@ EslTcp4ListenComplete (
> // Allocate a socket for this connection
> //
> ChildHandle = NULL;
> - pLayer = &mEslLayer;
> Status = EslSocketAllocate ( &ChildHandle,
> DEBUG_CONNECTION,
> &pNewSocket );
> @@ -1924,7 +1922,6 @@ EslTcp4TxBuffer (
> ESL_PACKET ** ppQueueHead;
> ESL_PACKET ** ppQueueTail;
> ESL_PACKET * pPreviousPacket;
> - ESL_TCP4_CONTEXT * pTcp4;
> size_t * pTxBytes;
> EFI_TCP4_TRANSMIT_DATA * pTxData;
> EFI_STATUS Status;
> @@ -1951,7 +1948,6 @@ EslTcp4TxBuffer (
> //
> // Determine the queue head
> //
> - pTcp4 = &pPort->Context.Tcp4;
> bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB ));
> bUrgentQueue = bUrgent
> && ( !pSocket->bOobInLine )
> diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c
> index 21c4109..77a4a4c 100644
> --- a/StdLib/EfiSocketLib/Tcp6.c
> +++ b/StdLib/EfiSocketLib/Tcp6.c
> @@ -871,7 +871,6 @@ EslTcp6ListenComplete (
> EFI_HANDLE ChildHandle;
> struct sockaddr_in6 LocalAddress;
> EFI_TCP6_CONFIG_DATA * pConfigData;
> - ESL_LAYER * pLayer;
> ESL_PORT * pNewPort;
> ESL_SOCKET * pNewSocket;
> ESL_SOCKET * pSocket;
> @@ -900,7 +899,6 @@ EslTcp6ListenComplete (
> // Allocate a socket for this connection
> //
> ChildHandle = NULL;
> - pLayer = &mEslLayer;
> Status = EslSocketAllocate ( &ChildHandle,
> DEBUG_CONNECTION,
> &pNewSocket );
> @@ -1993,7 +1991,6 @@ EslTcp6TxBuffer (
> ESL_PACKET ** ppQueueHead;
> ESL_PACKET ** ppQueueTail;
> ESL_PACKET * pPreviousPacket;
> - ESL_TCP6_CONTEXT * pTcp6;
> size_t * pTxBytes;
> EFI_TCP6_TRANSMIT_DATA * pTxData;
> EFI_STATUS Status;
> @@ -2020,7 +2017,6 @@ EslTcp6TxBuffer (
> //
> // Determine the queue head
> //
> - pTcp6 = &pPort->Context.Tcp6;
> bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB ));
> bUrgentQueue = bUrgent
> && ( !pSocket->bOobInLine )
> diff --git a/StdLib/LibC/Containers/Queues/Fifo.c
> b/StdLib/LibC/Containers/Queues/Fifo.c
> index 73254d2..93b62f8 100644
> --- a/StdLib/LibC/Containers/Queues/Fifo.c
> +++ b/StdLib/LibC/Containers/Queues/Fifo.c
> @@ -278,7 +278,6 @@ FIFO_Dequeue (
> {
> UINTN QPtr;
> UINT32 RDex;
> - UINT32 SizeOfElement;
> UINT32 i;
>
> assert(Self != NULL);
> @@ -289,7 +288,6 @@ FIFO_Dequeue (
> }
> else {
> RDex = Self->ReadIndex;
> // Get this FIFO's Read Index
> - SizeOfElement = Self->ElementSize;
> // Get size of this FIFO's elements
> Count = MIN(Count, Self->Count(Self, AsElements));
> // Lesser of requested or actual
>
> QPtr = (UINTN)Self->Queue + (RDex * Self->ElementSize);
> // Point to Read location in FIFO
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> index 1c978ea..b498d2b 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> @@ -75,7 +75,7 @@ IIO_GetInChar (
> {
> cIIO *This;
> cFIFO *InBuf;
> - EFI_STATUS Status;
> + size_t Status;
> ssize_t NumRead;
> wint_t RetVal;
> wchar_t InChar;
> @@ -92,8 +92,10 @@ IIO_GetInChar (
> }
> if(BufCnt > 0) {
> Status = InBuf->Read(InBuf, &InChar, 1);
> - --BufCnt;
> - NumRead = 1;
> + if (Status > 0) {
> + --BufCnt;
> + NumRead = 1;
> + }
> }
> else {
> NumRead = filp->f_ops->fo_read(filp, &filp->f_offset,
> sizeof(wchar_t), &InChar);
> @@ -104,7 +106,7 @@ IIO_GetInChar (
> else {
> RetVal = (wint_t)InChar;
> }
> - return InChar;
> + return RetVal;
> }
>
> /** Get the current cursor position.
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> index 927f4f4..2c5e81d 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> @@ -63,7 +63,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> UINT32 CurRow; // Current cursor row on the
> screen
> UINT32 PrevColumn; // Previous column. Used to
> detect wrapping.
> UINT32 AdjColumn; // Current cursor column on the
> screen
> - UINT32 AdjRow; // Current cursor row on the
> screen
>
> RetVal = -1;
> wcb = wc;
> @@ -79,7 +78,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> CurRow = This->CurrentXY.Row;
>
> numW = 1; // The majority of characters buffer one
> character
> - AdjRow = 0; // Most characters just cause horizontal
> movement
> AdjColumn = 0;
> if(OFlag & OPOST) {
> /* Perform output processing */
> @@ -127,7 +125,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> numW = 2;
> CurColumn = 0;
> }
> - AdjRow = 1;
> break; //}}
>
> case CHAR_BACKSPACE: //{{
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> index bcae9c8..3b226d1 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> @@ -37,7 +37,6 @@ IIO_NonCanonRead (
> cIIO *This;
> cFIFO *InBuf;
> struct termios *Termio;
> - EFI_STATUS Status;
> ssize_t NumRead;
> cc_t tioMin;
> cc_t tioTime;
> @@ -74,7 +73,7 @@ IIO_NonCanonRead (
> if(InBuf->IsEmpty(InBuf)) {
> NumRead = filp->f_ops->fo_read(filp, &filp->f_offset,
> sizeof(wchar_t), &InChar);
> if(NumRead > 0) {
> - Status = InBuf->Write(InBuf, &InChar, 1); // Buffer the
> character
> + InBuf->Write(InBuf, &InChar, 1); // Buffer the character
> }
> }
> // break;
> diff --git a/StdLib/LibC/Uefi/writev.c b/StdLib/LibC/Uefi/writev.c
> index 9cff086..56712c5 100644
> --- a/StdLib/LibC/Uefi/writev.c
> +++ b/StdLib/LibC/Uefi/writev.c
> @@ -101,7 +101,7 @@ writev(
> )
> {
> const struct iovec *pVecTmp;
> - char *pBuf, *pBufTmp;
> + char *pBuf;
> size_t TotalBytes, i, ret;
>
> //
> @@ -126,7 +126,7 @@ writev(
> // Copy vectors to the buffer
> //
>
> - for (pBufTmp = pBuf; iovcnt; iovcnt--) {
> + for (; iovcnt; iovcnt--) {
> bcopy(iov->iov_base, pBuf, iov->iov_len);
> pBuf += iov->iov_len;
> iov++;
> --
> 2.1.1
>
>
> -----------------------------------------------------------------------
> -------
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
>
> -----------------------------------------------------------------------
> -------
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Yes, I confirm all these errors where triggered by "set but not used"
warnings.
To answer the inline question about this pattern:
Status = pSocketProtocol->pfnListen ( pSocketProtocol,
backlog,
&errno );
+ if (EFI_ERROR (Status)) {
+ errno = -1;
+ }
The error was 'Status' is "set but not used".
Either I had to delete Status or I had to use it. I went for the second
option.
So, it looks the approach you would have prefer to use is to delete
'Status'. Should I submit a new patch for this one or you are ok to do it
yourself?
About '<DVM> Rejected. GETLONG(ttl, cp);'
You are right. But we will need a way to prevent the compiler to raise the
'set but not used' warning.
Note: they are also planning to add this warning message to the Intel GCC
toolchains.
> -----Original Message-----
> From: Mcdaniel, Daryl [mailto:daryl.mcdaniel@intel.com]
> Sent: 30 October 2014 01:05
> To: Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net
> Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to
> build with GCC
>
> Olivier,
>
> I have some questions about your patch submission. The majority are
> inserted, inline, into your quoted message below.
> Any patch marked '<DVM> Accepted.' has been committed as Reviewed by:
> Daryl McDaniel <daryl.mcdaniel@intel.com>.
>
> The remainder require further discussion. All of my comments are
> prefixed with <DVM>.
>
> <DVM> Were these all "set but not used" warnings? If not, what errors
> were you seeing and what version of GCC were you using?
>
> Daryl McDaniel
>
> -----Original Message-----
> From: Olivier Martin [mailto:olivier.martin@arm.com]
> Sent: Wednesday, October 29, 2014 3:01 AM
> To: Mcdaniel, Daryl
> Cc: edk2-devel@lists.sourceforge.net; Olivier Martin
> Subject: [PATCH] StdLib/LibC: Removed/Fixed unused variable to build
> with GCC
>
> Some of the variables are unused that trigger a GCC warning/error.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Olivier Martin <olivier.martin@arm.com>
> ---
> StdLib/BsdSocketLib/bind.c | 3 +++
> StdLib/BsdSocketLib/getnameinfo.c | 8 ++++++--
> StdLib/BsdSocketLib/getsockopt.c | 3 +++
> StdLib/BsdSocketLib/listen.c | 3 +++
> StdLib/BsdSocketLib/poll.c | 3 +++
> StdLib/BsdSocketLib/res_comp.c | 4 ++--
> StdLib/BsdSocketLib/res_mkupdate.c | 3 +--
> StdLib/BsdSocketLib/res_update.c | 4 ----
> StdLib/BsdSocketLib/setsockopt.c | 3 +++
> StdLib/EfiSocketLib/Ip4.c | 12 ------------
> StdLib/EfiSocketLib/Socket.c | 1 +
> StdLib/EfiSocketLib/Tcp4.c | 4 ----
> StdLib/EfiSocketLib/Tcp6.c | 4 ----
> StdLib/LibC/Containers/Queues/Fifo.c | 2 --
> StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c | 10 ++++++----
> StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c | 3 ---
> StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c | 3 +--
> StdLib/LibC/Uefi/writev.c | 4 ++--
> 18 files changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/StdLib/BsdSocketLib/bind.c b/StdLib/BsdSocketLib/bind.c
> index f544c94..f22fe2f 100644
> --- a/StdLib/BsdSocketLib/bind.c
> +++ b/StdLib/BsdSocketLib/bind.c
> @@ -65,6 +65,9 @@ bind (
> name,
> namelen,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
> <DVM> errno is set in the pfnBind call. Are you seeing that errors
> occur yet errno doesn't get set?
> Also, -1 is ERESTART which indicates that a system call needs to be
> restarted. Are you sure you don't want that instead of one of the
> other predefined errors?
>
> //
> diff --git a/StdLib/BsdSocketLib/getnameinfo.c
> b/StdLib/BsdSocketLib/getnameinfo.c
> index fab3460..0318d93 100644
> --- a/StdLib/BsdSocketLib/getnameinfo.c
> +++ b/StdLib/BsdSocketLib/getnameinfo.c
> @@ -411,15 +411,19 @@ ip6_sa2str(
> int flags
> )
> {
> - unsigned int ifindex;
> +#if 0
> + unsigned int ifindex;
> const struct in6_addr *a6;
> +#endif
> int n;
>
> _DIAGASSERT(sa6 != NULL);
> _DIAGASSERT(buf != NULL);
>
> - ifindex = (unsigned int)sa6->sin6_scope_id;
> +#if 0
> + ifindex = (unsigned int)sa6->sin6_scope_id;
> a6 = &sa6->sin6_addr;
> +#endif
>
> #ifdef NI_NUMERICSCOPE
> if ((flags & NI_NUMERICSCOPE) != 0) {
> <DVM> Accepted.
>
> diff --git a/StdLib/BsdSocketLib/getsockopt.c
> b/StdLib/BsdSocketLib/getsockopt.c
> index 47b7c6f..fdbd6f7 100644
> --- a/StdLib/BsdSocketLib/getsockopt.c
> +++ b/StdLib/BsdSocketLib/getsockopt.c
> @@ -60,6 +60,9 @@ getsockopt (
> option_value,
> option_len,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
> <DVM> Same comment as for bind(). pfnOptionGet() sets errno for all
> error conditions. It even sets errno to ESUCCESS, 0, if there are no
> errors.
> Please let me know if you are seeing getsockopt() failing but errno is
> not set.
>
> //
> diff --git a/StdLib/BsdSocketLib/listen.c
> b/StdLib/BsdSocketLib/listen.c index 7c6d5f3..d3060a4 100644
> --- a/StdLib/BsdSocketLib/listen.c
> +++ b/StdLib/BsdSocketLib/listen.c
> @@ -58,6 +58,9 @@ listen (
> Status = pSocketProtocol->pfnListen ( pSocketProtocol,
> backlog,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
> <DVM> Same comments as for bind() and getsockopt(). pfnListen() sets
> errno.
>
> //
> diff --git a/StdLib/BsdSocketLib/poll.c b/StdLib/BsdSocketLib/poll.c
> index dc17567..dcc367a 100644
> --- a/StdLib/BsdSocketLib/poll.c
> +++ b/StdLib/BsdSocketLib/poll.c
> @@ -49,6 +49,9 @@ BslSocketPoll (
> Events,
> &DetectedEvents,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
> <DVM> Like bind(), getsockopt(), and listen(), the internal pfnPoll()
> function sets errno if an error occurs. The only errno value this
> implementation supports is EINVAL which indicates that a non-socket or
> invalid fd was passed as the first argument. I noticed a bug in
> pfnPoll() where errno is not actually set to EINVAL. That will be
> fixed.
>
> //
> diff --git a/StdLib/BsdSocketLib/res_comp.c
> b/StdLib/BsdSocketLib/res_comp.c index cddda3e..7b5f2aa 100644
> --- a/StdLib/BsdSocketLib/res_comp.c
> +++ b/StdLib/BsdSocketLib/res_comp.c
> @@ -168,7 +168,7 @@ res_hnok(
> const char *dn
> )
> {
> - int ppch = '\0', pch = PERIOD, ch = *dn++;
> + int pch = PERIOD, ch = *dn++;
>
> while (ch != '\0') {
> int nch = *dn++;
> @@ -185,7 +185,7 @@ res_hnok(
> if (!middlechar(ch))
> return (0);
> }
> - ppch = pch, pch = ch, ch = nch;
> + pch = ch, ch = nch;
> }
> return (1);
> }
> <DVM> Accepted.
>
> diff --git a/StdLib/BsdSocketLib/res_mkupdate.c
> b/StdLib/BsdSocketLib/res_mkupdate.c
> index 3201f31..1056fc6 100644
> --- a/StdLib/BsdSocketLib/res_mkupdate.c
> +++ b/StdLib/BsdSocketLib/res_mkupdate.c
> @@ -100,7 +100,7 @@ int
> res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) {
> ns_updrec *rrecp_start = rrecp_in;
> HEADER *hp;
> - u_char *cp, *sp1, *sp2, *startp, *endp;
> + u_char *cp, *sp2, *startp, *endp;
> int n, i, soanum, multiline;
> ns_updrec *rrecp;
> struct in_addr ina;
> @@ -125,7 +125,6 @@ res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int
> buflen) {
> hp->id = htons(++_res.id);
> hp->opcode = ns_o_update;
> hp->rcode = NOERROR;
> - sp1 = buf + 2*INT16SZ; /* save pointer to zocount */
> cp = buf + HFIXEDSZ;
> buflen -= HFIXEDSZ;
> dpp = dnptrs;
> <DVM> Accepted
>
> diff --git a/StdLib/BsdSocketLib/res_update.c
> b/StdLib/BsdSocketLib/res_update.c
> index a01d832..f3ff0ce 100644
> --- a/StdLib/BsdSocketLib/res_update.c
> +++ b/StdLib/BsdSocketLib/res_update.c
> @@ -120,7 +120,6 @@ res_update(ns_updrec *rrecp_in) {
> int i, j, k = 0, n, ancount, nscount, arcount, rcode, rdatasize,
> newgroup, done, myzone, seen_before, numzones = 0;
> u_int16_t dlen, class, qclass, type, qtype;
> - u_int32_t ttl;
>
> if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
> h_errno = NETDB_INTERNAL;
> @@ -302,7 +301,6 @@ res_update(ns_updrec *rrecp_in) {
> if (cp + INT32SZ + INT16SZ > eom)
> return (-1);
> /* continue processing the soa record */
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> @@ -424,7 +422,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n",
> return (-1);
> GETSHORT(type, cp);
> GETSHORT(class, cp);
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> @@ -450,7 +447,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n",
> return (-1);
> GETSHORT(type, cp);
> GETSHORT(class, cp);
> - GETLONG(ttl, cp);
> GETSHORT(dlen, cp);
> if (cp + dlen > eom)
> return (-1);
> <DVM> Rejected. While ttl itself isn't used, the GETLONG(ttl, cp);
> invocations are required in order to advance the cp pointer over the
> ttl field of the header.
>
> diff --git a/StdLib/BsdSocketLib/setsockopt.c
> b/StdLib/BsdSocketLib/setsockopt.c
> index 64f3a35..7db7dea 100644
> --- a/StdLib/BsdSocketLib/setsockopt.c
> +++ b/StdLib/BsdSocketLib/setsockopt.c
> @@ -59,6 +59,9 @@ setsockopt (
> option_value,
> option_len,
> &errno );
> + if (EFI_ERROR (Status)) {
> + errno = -1;
> + }
> }
> <DVM> Like bind(), etc. pfnOptionSet() sets errno if any error occurs.
>
> //
> diff --git a/StdLib/EfiSocketLib/Ip4.c b/StdLib/EfiSocketLib/Ip4.c
> index ed71194..46425db 100644
> --- a/StdLib/EfiSocketLib/Ip4.c
> +++ b/StdLib/EfiSocketLib/Ip4.c
> @@ -242,8 +242,6 @@ EslIp4OptionSet (
> )
> {
> BOOLEAN bTrueFalse;
> - socklen_t LengthInBytes;
> - UINT8 * pOptionData;
> EFI_STATUS Status;
>
> DBG_ENTER ( );
> @@ -257,8 +255,6 @@ EslIp4OptionSet (
> //
> // Determine if the option protocol matches
> //
> - LengthInBytes = 0;
> - pOptionData = NULL;
> switch ( OptionName ) {
> default:
> //
> @@ -283,12 +279,6 @@ EslIp4OptionSet (
> bTrueFalse = FALSE;
> }
> pOptionValue = &bTrueFalse;
> -
> - //
> - // Set the option value
> - //
> - pOptionData = (UINT8 *)&pSocket->bIncludeHeader;
> - LengthInBytes = sizeof ( pSocket->bIncludeHeader );
> }
> break;
> }
> <DVM> The patch is valid as far as removing two meaningless variables.
> There is something else wrong with this function, though, since it does
> nothing other than validate that OptionName is set to IP_HDRINCL.
> Query submitted to sub-package owner.
>
> @@ -653,7 +643,6 @@ EslIp4RxComplete (
> )
> {
> size_t LengthInBytes;
> - ESL_PORT * pPort;
> ESL_PACKET * pPacket;
> EFI_IP4_RECEIVE_DATA * pRxData;
> EFI_STATUS Status;
> @@ -663,7 +652,6 @@ EslIp4RxComplete (
> //
> // Get the operation status.
> //
> - pPort = pIo->pPort;
> Status = pIo->Token.Ip4Rx.Status;
>
> //
> <DVM> Accepted change to EslIp4RxComplete().
>
> diff --git a/StdLib/EfiSocketLib/Socket.c
> b/StdLib/EfiSocketLib/Socket.c index d53473e..740602d 100644
> --- a/StdLib/EfiSocketLib/Socket.c
> +++ b/StdLib/EfiSocketLib/Socket.c
> @@ -4130,6 +4130,7 @@ EslSocketPortCloseComplete (
> //
> Status = EslSocketPortCloseRxDone ( pPort );
> DBG_EXIT_STATUS ( Status );
> + ASSERT_EFI_ERROR (Status);
> }
> <DVM> Query submitted to the sub-package owner. The ASSERT may be
> superfluous.
>
> diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c
> index 7ece38d..0419ee2 100644
> --- a/StdLib/EfiSocketLib/Tcp4.c
> +++ b/StdLib/EfiSocketLib/Tcp4.c
> @@ -840,7 +840,6 @@ EslTcp4ListenComplete (
> EFI_HANDLE ChildHandle;
> struct sockaddr_in LocalAddress;
> EFI_TCP4_CONFIG_DATA * pConfigData;
> - ESL_LAYER * pLayer;
> ESL_PORT * pNewPort;
> ESL_SOCKET * pNewSocket;
> ESL_SOCKET * pSocket;
> @@ -869,7 +868,6 @@ EslTcp4ListenComplete (
> // Allocate a socket for this connection
> //
> ChildHandle = NULL;
> - pLayer = &mEslLayer;
> Status = EslSocketAllocate ( &ChildHandle,
> DEBUG_CONNECTION,
> &pNewSocket );
> <DVM> Accepted.
>
> @@ -1924,7 +1922,6 @@ EslTcp4TxBuffer (
> ESL_PACKET ** ppQueueHead;
> ESL_PACKET ** ppQueueTail;
> ESL_PACKET * pPreviousPacket;
> - ESL_TCP4_CONTEXT * pTcp4;
> size_t * pTxBytes;
> EFI_TCP4_TRANSMIT_DATA * pTxData;
> EFI_STATUS Status;
> @@ -1951,7 +1948,6 @@ EslTcp4TxBuffer (
> //
> // Determine the queue head
> //
> - pTcp4 = &pPort->Context.Tcp4;
> bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB ));
> bUrgentQueue = bUrgent
> && ( !pSocket->bOobInLine )
> <DVM> Accepted.
>
> diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c
> index 21c4109..77a4a4c 100644
> --- a/StdLib/EfiSocketLib/Tcp6.c
> +++ b/StdLib/EfiSocketLib/Tcp6.c
> @@ -871,7 +871,6 @@ EslTcp6ListenComplete (
> EFI_HANDLE ChildHandle;
> struct sockaddr_in6 LocalAddress;
> EFI_TCP6_CONFIG_DATA * pConfigData;
> - ESL_LAYER * pLayer;
> ESL_PORT * pNewPort;
> ESL_SOCKET * pNewSocket;
> ESL_SOCKET * pSocket;
> @@ -900,7 +899,6 @@ EslTcp6ListenComplete (
> // Allocate a socket for this connection
> //
> ChildHandle = NULL;
> - pLayer = &mEslLayer;
> Status = EslSocketAllocate ( &ChildHandle,
> DEBUG_CONNECTION,
> &pNewSocket );
> <DVM> Accepted.
>
> @@ -1993,7 +1991,6 @@ EslTcp6TxBuffer (
> ESL_PACKET ** ppQueueHead;
> ESL_PACKET ** ppQueueTail;
> ESL_PACKET * pPreviousPacket;
> - ESL_TCP6_CONTEXT * pTcp6;
> size_t * pTxBytes;
> EFI_TCP6_TRANSMIT_DATA * pTxData;
> EFI_STATUS Status;
>
> @@ -2020,7 +2017,6 @@ EslTcp6TxBuffer (
> //
> // Determine the queue head
> //
> - pTcp6 = &pPort->Context.Tcp6;
> bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB ));
> bUrgentQueue = bUrgent
> && ( !pSocket->bOobInLine )
> <DVM> Accepted.
>
> diff --git a/StdLib/LibC/Containers/Queues/Fifo.c
> b/StdLib/LibC/Containers/Queues/Fifo.c
> index 73254d2..93b62f8 100644
> --- a/StdLib/LibC/Containers/Queues/Fifo.c
> +++ b/StdLib/LibC/Containers/Queues/Fifo.c
> @@ -278,7 +278,6 @@ FIFO_Dequeue (
> {
> UINTN QPtr;
> UINT32 RDex;
> - UINT32 SizeOfElement;
> UINT32 i;
>
> assert(Self != NULL);
> @@ -289,7 +288,6 @@ FIFO_Dequeue (
> }
> else {
> RDex = Self->ReadIndex;
> // Get this FIFO's Read Index
> - SizeOfElement = Self->ElementSize;
> // Get size of this FIFO's elements
> Count = MIN(Count, Self->Count(Self, AsElements));
> // Lesser of requested or actual
>
> QPtr = (UINTN)Self->Queue + (RDex * Self->ElementSize);
> // Point to Read location in FIFO
> <DVM> Replacing all of the "Self->ElementSize" clauses in the function
> with "SizeOfElement" would be more efficient.
>
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> index 1c978ea..b498d2b 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c
> @@ -75,7 +75,7 @@ IIO_GetInChar (
> {
> cIIO *This;
> cFIFO *InBuf;
> - EFI_STATUS Status;
> + size_t Status;
> ssize_t NumRead;
> wint_t RetVal;
> wchar_t InChar;
> @@ -92,8 +92,10 @@ IIO_GetInChar (
> }
> if(BufCnt > 0) {
> Status = InBuf->Read(InBuf, &InChar, 1);
> - --BufCnt;
> - NumRead = 1;
> + if (Status > 0) {
> + --BufCnt;
> + NumRead = 1;
> + }
> }
> else {
> NumRead = filp->f_ops->fo_read(filp, &filp->f_offset,
> sizeof(wchar_t), &InChar); @@ -104,7 +106,7 @@ IIO_GetInChar (
> else {
> RetVal = (wint_t)InChar;
> }
> - return InChar;
> + return RetVal;
> }
>
> /** Get the current cursor position.
> <DVM> Accepted.
>
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> index 927f4f4..2c5e81d 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c
> @@ -63,7 +63,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> UINT32 CurRow; // Current cursor row on the
> screen
> UINT32 PrevColumn; // Previous column. Used to
> detect wrapping.
> UINT32 AdjColumn; // Current cursor column on the
> screen
> - UINT32 AdjRow; // Current cursor row on the
> screen
>
> RetVal = -1;
> wcb = wc;
> @@ -79,7 +78,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> CurRow = This->CurrentXY.Row;
>
> numW = 1; // The majority of characters buffer one
> character
> - AdjRow = 0; // Most characters just cause horizontal
> movement
> AdjColumn = 0;
> if(OFlag & OPOST) {
> /* Perform output processing */
> @@ -127,7 +125,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf,
> wchar_t InCh)
> numW = 2;
> CurColumn = 0;
> }
> - AdjRow = 1;
> break; //}}
>
> case CHAR_BACKSPACE: //{{
> <DVM> Accepted.
>
> diff --git a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> index bcae9c8..3b226d1 100644
> --- a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> +++ b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c
> @@ -37,7 +37,6 @@ IIO_NonCanonRead (
> cIIO *This;
> cFIFO *InBuf;
> struct termios *Termio;
> - EFI_STATUS Status;
> ssize_t NumRead;
> cc_t tioMin;
> cc_t tioTime;
> @@ -74,7 +73,7 @@ IIO_NonCanonRead (
> if(InBuf->IsEmpty(InBuf)) {
> NumRead = filp->f_ops->fo_read(filp, &filp->f_offset,
> sizeof(wchar_t), &InChar);
> if(NumRead > 0) {
> - Status = InBuf->Write(InBuf, &InChar, 1); // Buffer the
> character
> + InBuf->Write(InBuf, &InChar, 1); // Buffer the character
> }
> }
> // break;
> <DVM> Accepted.
>
> diff --git a/StdLib/LibC/Uefi/writev.c b/StdLib/LibC/Uefi/writev.c
> index 9cff086..56712c5 100644
> --- a/StdLib/LibC/Uefi/writev.c
> +++ b/StdLib/LibC/Uefi/writev.c
> @@ -101,7 +101,7 @@ writev(
> )
> {
> const struct iovec *pVecTmp;
> - char *pBuf, *pBufTmp;
> + char *pBuf;
> size_t TotalBytes, i, ret;
>
> //
> @@ -126,7 +126,7 @@ writev(
> // Copy vectors to the buffer
> //
>
> - for (pBufTmp = pBuf; iovcnt; iovcnt--) {
> + for (; iovcnt; iovcnt--) {
> bcopy(iov->iov_base, pBuf, iov->iov_len);
> pBuf += iov->iov_len;
> iov++;
> <DVM> Accepted.
> --
> 2.1.1
>
------------------------------------------------------------------------------
Olivier, Thank you for the confirmation and the additional background on the errno=-1 changes. I was a bit "confused" by your second mail which I interpreted as meaning that you were trying to guarantee that errno was set. I'll handle getting rid of the "'Status' set but not used" error. Would you like to review my changes before I commit them? I'm fine either way. In case I didn't say it before, I am grateful for the effort you put in to reporting these problems and providing the patch information. Thank You! Gratefully yours, Daryl McDaniel -----Original Message----- From: Olivier Martin [mailto:olivier.martin@arm.com] Sent: Thursday, October 30, 2014 3:21 AM To: Mcdaniel, Daryl Cc: edk2-devel@lists.sourceforge.net Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to build with GCC Yes, I confirm all these errors where triggered by "set but not used" warnings. To answer the inline question about this pattern: Status = pSocketProtocol->pfnListen ( pSocketProtocol, backlog, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } The error was 'Status' is "set but not used". Either I had to delete Status or I had to use it. I went for the second option. So, it looks the approach you would have prefer to use is to delete 'Status'. Should I submit a new patch for this one or you are ok to do it yourself? About '<DVM> Rejected. GETLONG(ttl, cp);' You are right. But we will need a way to prevent the compiler to raise the 'set but not used' warning. Note: they are also planning to add this warning message to the Intel GCC toolchains. > -----Original Message----- > From: Mcdaniel, Daryl [mailto:daryl.mcdaniel@intel.com] > Sent: 30 October 2014 01:05 > To: Olivier Martin > Cc: edk2-devel@lists.sourceforge.net > Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to > build with GCC > > Olivier, > > I have some questions about your patch submission. The majority are > inserted, inline, into your quoted message below. > Any patch marked '<DVM> Accepted.' has been committed as Reviewed by: > Daryl McDaniel <daryl.mcdaniel@intel.com>. > > The remainder require further discussion. All of my comments are > prefixed with <DVM>. > > <DVM> Were these all "set but not used" warnings? If not, what errors > were you seeing and what version of GCC were you using? > > Daryl McDaniel > > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Wednesday, October 29, 2014 3:01 AM > To: Mcdaniel, Daryl > Cc: edk2-devel@lists.sourceforge.net; Olivier Martin > Subject: [PATCH] StdLib/LibC: Removed/Fixed unused variable to build > with GCC > > Some of the variables are unused that trigger a GCC warning/error. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > --- > StdLib/BsdSocketLib/bind.c | 3 +++ > StdLib/BsdSocketLib/getnameinfo.c | 8 ++++++-- > StdLib/BsdSocketLib/getsockopt.c | 3 +++ > StdLib/BsdSocketLib/listen.c | 3 +++ > StdLib/BsdSocketLib/poll.c | 3 +++ > StdLib/BsdSocketLib/res_comp.c | 4 ++-- > StdLib/BsdSocketLib/res_mkupdate.c | 3 +-- > StdLib/BsdSocketLib/res_update.c | 4 ---- > StdLib/BsdSocketLib/setsockopt.c | 3 +++ > StdLib/EfiSocketLib/Ip4.c | 12 ------------ > StdLib/EfiSocketLib/Socket.c | 1 + > StdLib/EfiSocketLib/Tcp4.c | 4 ---- > StdLib/EfiSocketLib/Tcp6.c | 4 ---- > StdLib/LibC/Containers/Queues/Fifo.c | 2 -- > StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c | 10 ++++++---- > StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c | 3 --- > StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c | 3 +-- > StdLib/LibC/Uefi/writev.c | 4 ++-- > 18 files changed, 34 insertions(+), 43 deletions(-) > > diff --git a/StdLib/BsdSocketLib/bind.c b/StdLib/BsdSocketLib/bind.c > index f544c94..f22fe2f 100644 > --- a/StdLib/BsdSocketLib/bind.c > +++ b/StdLib/BsdSocketLib/bind.c > @@ -65,6 +65,9 @@ bind ( > name, > namelen, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > } > <DVM> errno is set in the pfnBind call. Are you seeing that errors > occur yet errno doesn't get set? > Also, -1 is ERESTART which indicates that a system call needs to be > restarted. Are you sure you don't want that instead of one of the > other predefined errors? > > // > diff --git a/StdLib/BsdSocketLib/getnameinfo.c > b/StdLib/BsdSocketLib/getnameinfo.c > index fab3460..0318d93 100644 > --- a/StdLib/BsdSocketLib/getnameinfo.c > +++ b/StdLib/BsdSocketLib/getnameinfo.c > @@ -411,15 +411,19 @@ ip6_sa2str( > int flags > ) > { > - unsigned int ifindex; > +#if 0 > + unsigned int ifindex; > const struct in6_addr *a6; > +#endif > int n; > > _DIAGASSERT(sa6 != NULL); > _DIAGASSERT(buf != NULL); > > - ifindex = (unsigned int)sa6->sin6_scope_id; > +#if 0 > + ifindex = (unsigned int)sa6->sin6_scope_id; > a6 = &sa6->sin6_addr; > +#endif > > #ifdef NI_NUMERICSCOPE > if ((flags & NI_NUMERICSCOPE) != 0) { > <DVM> Accepted. > > diff --git a/StdLib/BsdSocketLib/getsockopt.c > b/StdLib/BsdSocketLib/getsockopt.c > index 47b7c6f..fdbd6f7 100644 > --- a/StdLib/BsdSocketLib/getsockopt.c > +++ b/StdLib/BsdSocketLib/getsockopt.c > @@ -60,6 +60,9 @@ getsockopt ( > option_value, > option_len, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > } > <DVM> Same comment as for bind(). pfnOptionGet() sets errno for all > error conditions. It even sets errno to ESUCCESS, 0, if there are no > errors. > Please let me know if you are seeing getsockopt() failing but errno is > not set. > > // > diff --git a/StdLib/BsdSocketLib/listen.c > b/StdLib/BsdSocketLib/listen.c index 7c6d5f3..d3060a4 100644 > --- a/StdLib/BsdSocketLib/listen.c > +++ b/StdLib/BsdSocketLib/listen.c > @@ -58,6 +58,9 @@ listen ( > Status = pSocketProtocol->pfnListen ( pSocketProtocol, > backlog, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > } > <DVM> Same comments as for bind() and getsockopt(). pfnListen() sets > errno. > > // > diff --git a/StdLib/BsdSocketLib/poll.c b/StdLib/BsdSocketLib/poll.c > index dc17567..dcc367a 100644 > --- a/StdLib/BsdSocketLib/poll.c > +++ b/StdLib/BsdSocketLib/poll.c > @@ -49,6 +49,9 @@ BslSocketPoll ( > Events, > &DetectedEvents, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > } > <DVM> Like bind(), getsockopt(), and listen(), the internal pfnPoll() > function sets errno if an error occurs. The only errno value this > implementation supports is EINVAL which indicates that a non-socket or > invalid fd was passed as the first argument. I noticed a bug in > pfnPoll() where errno is not actually set to EINVAL. That will be > fixed. > > // > diff --git a/StdLib/BsdSocketLib/res_comp.c > b/StdLib/BsdSocketLib/res_comp.c index cddda3e..7b5f2aa 100644 > --- a/StdLib/BsdSocketLib/res_comp.c > +++ b/StdLib/BsdSocketLib/res_comp.c > @@ -168,7 +168,7 @@ res_hnok( > const char *dn > ) > { > - int ppch = '\0', pch = PERIOD, ch = *dn++; > + int pch = PERIOD, ch = *dn++; > > while (ch != '\0') { > int nch = *dn++; > @@ -185,7 +185,7 @@ res_hnok( > if (!middlechar(ch)) > return (0); > } > - ppch = pch, pch = ch, ch = nch; > + pch = ch, ch = nch; > } > return (1); > } > <DVM> Accepted. > > diff --git a/StdLib/BsdSocketLib/res_mkupdate.c > b/StdLib/BsdSocketLib/res_mkupdate.c > index 3201f31..1056fc6 100644 > --- a/StdLib/BsdSocketLib/res_mkupdate.c > +++ b/StdLib/BsdSocketLib/res_mkupdate.c > @@ -100,7 +100,7 @@ int > res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) { > ns_updrec *rrecp_start = rrecp_in; > HEADER *hp; > - u_char *cp, *sp1, *sp2, *startp, *endp; > + u_char *cp, *sp2, *startp, *endp; > int n, i, soanum, multiline; > ns_updrec *rrecp; > struct in_addr ina; > @@ -125,7 +125,6 @@ res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int > buflen) { > hp->id = htons(++_res.id); > hp->opcode = ns_o_update; > hp->rcode = NOERROR; > - sp1 = buf + 2*INT16SZ; /* save pointer to zocount */ > cp = buf + HFIXEDSZ; > buflen -= HFIXEDSZ; > dpp = dnptrs; > <DVM> Accepted > > diff --git a/StdLib/BsdSocketLib/res_update.c > b/StdLib/BsdSocketLib/res_update.c > index a01d832..f3ff0ce 100644 > --- a/StdLib/BsdSocketLib/res_update.c > +++ b/StdLib/BsdSocketLib/res_update.c > @@ -120,7 +120,6 @@ res_update(ns_updrec *rrecp_in) { > int i, j, k = 0, n, ancount, nscount, arcount, rcode, rdatasize, > newgroup, done, myzone, seen_before, numzones = 0; > u_int16_t dlen, class, qclass, type, qtype; > - u_int32_t ttl; > > if ((_res.options & RES_INIT) == 0 && res_init() == -1) { > h_errno = NETDB_INTERNAL; > @@ -302,7 +301,6 @@ res_update(ns_updrec *rrecp_in) { > if (cp + INT32SZ + INT16SZ > eom) > return (-1); > /* continue processing the soa record */ > - GETLONG(ttl, cp); > GETSHORT(dlen, cp); > if (cp + dlen > eom) > return (-1); > @@ -424,7 +422,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", > return (-1); > GETSHORT(type, cp); > GETSHORT(class, cp); > - GETLONG(ttl, cp); > GETSHORT(dlen, cp); > if (cp + dlen > eom) > return (-1); > @@ -450,7 +447,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", > return (-1); > GETSHORT(type, cp); > GETSHORT(class, cp); > - GETLONG(ttl, cp); > GETSHORT(dlen, cp); > if (cp + dlen > eom) > return (-1); > <DVM> Rejected. While ttl itself isn't used, the GETLONG(ttl, cp); > invocations are required in order to advance the cp pointer over the > ttl field of the header. > > diff --git a/StdLib/BsdSocketLib/setsockopt.c > b/StdLib/BsdSocketLib/setsockopt.c > index 64f3a35..7db7dea 100644 > --- a/StdLib/BsdSocketLib/setsockopt.c > +++ b/StdLib/BsdSocketLib/setsockopt.c > @@ -59,6 +59,9 @@ setsockopt ( > option_value, > option_len, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > } > <DVM> Like bind(), etc. pfnOptionSet() sets errno if any error occurs. > > // > diff --git a/StdLib/EfiSocketLib/Ip4.c b/StdLib/EfiSocketLib/Ip4.c > index ed71194..46425db 100644 > --- a/StdLib/EfiSocketLib/Ip4.c > +++ b/StdLib/EfiSocketLib/Ip4.c > @@ -242,8 +242,6 @@ EslIp4OptionSet ( > ) > { > BOOLEAN bTrueFalse; > - socklen_t LengthInBytes; > - UINT8 * pOptionData; > EFI_STATUS Status; > > DBG_ENTER ( ); > @@ -257,8 +255,6 @@ EslIp4OptionSet ( > // > // Determine if the option protocol matches > // > - LengthInBytes = 0; > - pOptionData = NULL; > switch ( OptionName ) { > default: > // > @@ -283,12 +279,6 @@ EslIp4OptionSet ( > bTrueFalse = FALSE; > } > pOptionValue = &bTrueFalse; > - > - // > - // Set the option value > - // > - pOptionData = (UINT8 *)&pSocket->bIncludeHeader; > - LengthInBytes = sizeof ( pSocket->bIncludeHeader ); > } > break; > } > <DVM> The patch is valid as far as removing two meaningless variables. > There is something else wrong with this function, though, since it does > nothing other than validate that OptionName is set to IP_HDRINCL. > Query submitted to sub-package owner. > > @@ -653,7 +643,6 @@ EslIp4RxComplete ( > ) > { > size_t LengthInBytes; > - ESL_PORT * pPort; > ESL_PACKET * pPacket; > EFI_IP4_RECEIVE_DATA * pRxData; > EFI_STATUS Status; > @@ -663,7 +652,6 @@ EslIp4RxComplete ( > // > // Get the operation status. > // > - pPort = pIo->pPort; > Status = pIo->Token.Ip4Rx.Status; > > // > <DVM> Accepted change to EslIp4RxComplete(). > > diff --git a/StdLib/EfiSocketLib/Socket.c > b/StdLib/EfiSocketLib/Socket.c index d53473e..740602d 100644 > --- a/StdLib/EfiSocketLib/Socket.c > +++ b/StdLib/EfiSocketLib/Socket.c > @@ -4130,6 +4130,7 @@ EslSocketPortCloseComplete ( > // > Status = EslSocketPortCloseRxDone ( pPort ); > DBG_EXIT_STATUS ( Status ); > + ASSERT_EFI_ERROR (Status); > } > <DVM> Query submitted to the sub-package owner. The ASSERT may be > superfluous. > > diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c > index 7ece38d..0419ee2 100644 > --- a/StdLib/EfiSocketLib/Tcp4.c > +++ b/StdLib/EfiSocketLib/Tcp4.c > @@ -840,7 +840,6 @@ EslTcp4ListenComplete ( > EFI_HANDLE ChildHandle; > struct sockaddr_in LocalAddress; > EFI_TCP4_CONFIG_DATA * pConfigData; > - ESL_LAYER * pLayer; > ESL_PORT * pNewPort; > ESL_SOCKET * pNewSocket; > ESL_SOCKET * pSocket; > @@ -869,7 +868,6 @@ EslTcp4ListenComplete ( > // Allocate a socket for this connection > // > ChildHandle = NULL; > - pLayer = &mEslLayer; > Status = EslSocketAllocate ( &ChildHandle, > DEBUG_CONNECTION, > &pNewSocket ); > <DVM> Accepted. > > @@ -1924,7 +1922,6 @@ EslTcp4TxBuffer ( > ESL_PACKET ** ppQueueHead; > ESL_PACKET ** ppQueueTail; > ESL_PACKET * pPreviousPacket; > - ESL_TCP4_CONTEXT * pTcp4; > size_t * pTxBytes; > EFI_TCP4_TRANSMIT_DATA * pTxData; > EFI_STATUS Status; > @@ -1951,7 +1948,6 @@ EslTcp4TxBuffer ( > // > // Determine the queue head > // > - pTcp4 = &pPort->Context.Tcp4; > bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); > bUrgentQueue = bUrgent > && ( !pSocket->bOobInLine ) > <DVM> Accepted. > > diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c > index 21c4109..77a4a4c 100644 > --- a/StdLib/EfiSocketLib/Tcp6.c > +++ b/StdLib/EfiSocketLib/Tcp6.c > @@ -871,7 +871,6 @@ EslTcp6ListenComplete ( > EFI_HANDLE ChildHandle; > struct sockaddr_in6 LocalAddress; > EFI_TCP6_CONFIG_DATA * pConfigData; > - ESL_LAYER * pLayer; > ESL_PORT * pNewPort; > ESL_SOCKET * pNewSocket; > ESL_SOCKET * pSocket; > @@ -900,7 +899,6 @@ EslTcp6ListenComplete ( > // Allocate a socket for this connection > // > ChildHandle = NULL; > - pLayer = &mEslLayer; > Status = EslSocketAllocate ( &ChildHandle, > DEBUG_CONNECTION, > &pNewSocket ); > <DVM> Accepted. > > @@ -1993,7 +1991,6 @@ EslTcp6TxBuffer ( > ESL_PACKET ** ppQueueHead; > ESL_PACKET ** ppQueueTail; > ESL_PACKET * pPreviousPacket; > - ESL_TCP6_CONTEXT * pTcp6; > size_t * pTxBytes; > EFI_TCP6_TRANSMIT_DATA * pTxData; > EFI_STATUS Status; > > @@ -2020,7 +2017,6 @@ EslTcp6TxBuffer ( > // > // Determine the queue head > // > - pTcp6 = &pPort->Context.Tcp6; > bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); > bUrgentQueue = bUrgent > && ( !pSocket->bOobInLine ) > <DVM> Accepted. > > diff --git a/StdLib/LibC/Containers/Queues/Fifo.c > b/StdLib/LibC/Containers/Queues/Fifo.c > index 73254d2..93b62f8 100644 > --- a/StdLib/LibC/Containers/Queues/Fifo.c > +++ b/StdLib/LibC/Containers/Queues/Fifo.c > @@ -278,7 +278,6 @@ FIFO_Dequeue ( > { > UINTN QPtr; > UINT32 RDex; > - UINT32 SizeOfElement; > UINT32 i; > > assert(Self != NULL); > @@ -289,7 +288,6 @@ FIFO_Dequeue ( > } > else { > RDex = Self->ReadIndex; > // Get this FIFO's Read Index > - SizeOfElement = Self->ElementSize; > // Get size of this FIFO's elements > Count = MIN(Count, Self->Count(Self, AsElements)); > // Lesser of requested or actual > > QPtr = (UINTN)Self->Queue + (RDex * Self->ElementSize); > // Point to Read location in FIFO > <DVM> Replacing all of the "Self->ElementSize" clauses in the function > with "SizeOfElement" would be more efficient. > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > index 1c978ea..b498d2b 100644 > --- a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > @@ -75,7 +75,7 @@ IIO_GetInChar ( > { > cIIO *This; > cFIFO *InBuf; > - EFI_STATUS Status; > + size_t Status; > ssize_t NumRead; > wint_t RetVal; > wchar_t InChar; > @@ -92,8 +92,10 @@ IIO_GetInChar ( > } > if(BufCnt > 0) { > Status = InBuf->Read(InBuf, &InChar, 1); > - --BufCnt; > - NumRead = 1; > + if (Status > 0) { > + --BufCnt; > + NumRead = 1; > + } > } > else { > NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, > sizeof(wchar_t), &InChar); @@ -104,7 +106,7 @@ IIO_GetInChar ( > else { > RetVal = (wint_t)InChar; > } > - return InChar; > + return RetVal; > } > > /** Get the current cursor position. > <DVM> Accepted. > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > index 927f4f4..2c5e81d 100644 > --- a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > @@ -63,7 +63,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > wchar_t InCh) > UINT32 CurRow; // Current cursor row on the > screen > UINT32 PrevColumn; // Previous column. Used to > detect wrapping. > UINT32 AdjColumn; // Current cursor column on the > screen > - UINT32 AdjRow; // Current cursor row on the > screen > > RetVal = -1; > wcb = wc; > @@ -79,7 +78,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > wchar_t InCh) > CurRow = This->CurrentXY.Row; > > numW = 1; // The majority of characters buffer one > character > - AdjRow = 0; // Most characters just cause horizontal > movement > AdjColumn = 0; > if(OFlag & OPOST) { > /* Perform output processing */ > @@ -127,7 +125,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > wchar_t InCh) > numW = 2; > CurColumn = 0; > } > - AdjRow = 1; > break; //}} > > case CHAR_BACKSPACE: //{{ > <DVM> Accepted. > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > index bcae9c8..3b226d1 100644 > --- a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > +++ b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > @@ -37,7 +37,6 @@ IIO_NonCanonRead ( > cIIO *This; > cFIFO *InBuf; > struct termios *Termio; > - EFI_STATUS Status; > ssize_t NumRead; > cc_t tioMin; > cc_t tioTime; > @@ -74,7 +73,7 @@ IIO_NonCanonRead ( > if(InBuf->IsEmpty(InBuf)) { > NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, > sizeof(wchar_t), &InChar); > if(NumRead > 0) { > - Status = InBuf->Write(InBuf, &InChar, 1); // Buffer the > character > + InBuf->Write(InBuf, &InChar, 1); // Buffer the character > } > } > // break; > <DVM> Accepted. > > diff --git a/StdLib/LibC/Uefi/writev.c b/StdLib/LibC/Uefi/writev.c > index 9cff086..56712c5 100644 > --- a/StdLib/LibC/Uefi/writev.c > +++ b/StdLib/LibC/Uefi/writev.c > @@ -101,7 +101,7 @@ writev( > ) > { > const struct iovec *pVecTmp; > - char *pBuf, *pBufTmp; > + char *pBuf; > size_t TotalBytes, i, ret; > > // > @@ -126,7 +126,7 @@ writev( > // Copy vectors to the buffer > // > > - for (pBufTmp = pBuf; iovcnt; iovcnt--) { > + for (; iovcnt; iovcnt--) { > bcopy(iov->iov_base, pBuf, iov->iov_len); > pBuf += iov->iov_len; > iov++; > <DVM> Accepted. > -- > 2.1.1 > ------------------------------------------------------------------------------
Go ahead with removing 'Status'. I do not need to review this patch. Thanks, Olivier > -----Original Message----- > From: Mcdaniel, Daryl [mailto:daryl.mcdaniel@intel.com] > Sent: 30 October 2014 17:45 > To: Olivier Martin > Cc: edk2-devel@lists.sourceforge.net > Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to > build with GCC > > Olivier, > > Thank you for the confirmation and the additional background on the > errno=-1 changes. I was a bit "confused" by your second mail which I > interpreted as meaning that you were trying to guarantee that errno was > set. > > I'll handle getting rid of the "'Status' set but not used" error. > Would you like to review my changes before I commit them? I'm fine > either way. > > In case I didn't say it before, I am grateful for the effort you put in > to reporting these problems and providing the patch information. Thank > You! > > Gratefully yours, > Daryl McDaniel > > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Thursday, October 30, 2014 3:21 AM > To: Mcdaniel, Daryl > Cc: edk2-devel@lists.sourceforge.net > Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to > build with GCC > > Yes, I confirm all these errors where triggered by "set but not used" > warnings. > > To answer the inline question about this pattern: > Status = pSocketProtocol->pfnListen ( pSocketProtocol, > backlog, > &errno ); > + if (EFI_ERROR (Status)) { > + errno = -1; > + } > > The error was 'Status' is "set but not used". > Either I had to delete Status or I had to use it. I went for the second > option. > So, it looks the approach you would have prefer to use is to delete > 'Status'. Should I submit a new patch for this one or you are ok to do > it > yourself? > > > About '<DVM> Rejected. GETLONG(ttl, cp);' > You are right. But we will need a way to prevent the compiler to raise > the > 'set but not used' warning. > Note: they are also planning to add this warning message to the Intel > GCC > toolchains. > > > -----Original Message----- > > From: Mcdaniel, Daryl [mailto:daryl.mcdaniel@intel.com] > > Sent: 30 October 2014 01:05 > > To: Olivier Martin > > Cc: edk2-devel@lists.sourceforge.net > > Subject: RE: [PATCH] StdLib/LibC: Removed/Fixed unused variable to > > build with GCC > > > > Olivier, > > > > I have some questions about your patch submission. The majority are > > inserted, inline, into your quoted message below. > > Any patch marked '<DVM> Accepted.' has been committed as Reviewed by: > > Daryl McDaniel <daryl.mcdaniel@intel.com>. > > > > The remainder require further discussion. All of my comments are > > prefixed with <DVM>. > > > > <DVM> Were these all "set but not used" warnings? If not, what > errors > > were you seeing and what version of GCC were you using? > > > > Daryl McDaniel > > > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: Wednesday, October 29, 2014 3:01 AM > > To: Mcdaniel, Daryl > > Cc: edk2-devel@lists.sourceforge.net; Olivier Martin > > Subject: [PATCH] StdLib/LibC: Removed/Fixed unused variable to build > > with GCC > > > > Some of the variables are unused that trigger a GCC warning/error. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > --- > > StdLib/BsdSocketLib/bind.c | 3 +++ > > StdLib/BsdSocketLib/getnameinfo.c | 8 ++++++-- > > StdLib/BsdSocketLib/getsockopt.c | 3 +++ > > StdLib/BsdSocketLib/listen.c | 3 +++ > > StdLib/BsdSocketLib/poll.c | 3 +++ > > StdLib/BsdSocketLib/res_comp.c | 4 ++-- > > StdLib/BsdSocketLib/res_mkupdate.c | 3 +-- > > StdLib/BsdSocketLib/res_update.c | 4 ---- > > StdLib/BsdSocketLib/setsockopt.c | 3 +++ > > StdLib/EfiSocketLib/Ip4.c | 12 ------------ > > StdLib/EfiSocketLib/Socket.c | 1 + > > StdLib/EfiSocketLib/Tcp4.c | 4 ---- > > StdLib/EfiSocketLib/Tcp6.c | 4 ---- > > StdLib/LibC/Containers/Queues/Fifo.c | 2 -- > > StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c | 10 ++++++---- > > StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c | 3 --- > > StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c | 3 +-- > > StdLib/LibC/Uefi/writev.c | 4 ++-- > > 18 files changed, 34 insertions(+), 43 deletions(-) > > > > diff --git a/StdLib/BsdSocketLib/bind.c b/StdLib/BsdSocketLib/bind.c > > index f544c94..f22fe2f 100644 > > --- a/StdLib/BsdSocketLib/bind.c > > +++ b/StdLib/BsdSocketLib/bind.c > > @@ -65,6 +65,9 @@ bind ( > > name, > > namelen, > > &errno ); > > + if (EFI_ERROR (Status)) { > > + errno = -1; > > + } > > } > > <DVM> errno is set in the pfnBind call. Are you seeing that errors > > occur yet errno doesn't get set? > > Also, -1 is ERESTART which indicates that a system call needs to be > > restarted. Are you sure you don't want that instead of one of the > > other predefined errors? > > > > // > > diff --git a/StdLib/BsdSocketLib/getnameinfo.c > > b/StdLib/BsdSocketLib/getnameinfo.c > > index fab3460..0318d93 100644 > > --- a/StdLib/BsdSocketLib/getnameinfo.c > > +++ b/StdLib/BsdSocketLib/getnameinfo.c > > @@ -411,15 +411,19 @@ ip6_sa2str( > > int flags > > ) > > { > > - unsigned int ifindex; > > +#if 0 > > + unsigned int ifindex; > > const struct in6_addr *a6; > > +#endif > > int n; > > > > _DIAGASSERT(sa6 != NULL); > > _DIAGASSERT(buf != NULL); > > > > - ifindex = (unsigned int)sa6->sin6_scope_id; > > +#if 0 > > + ifindex = (unsigned int)sa6->sin6_scope_id; > > a6 = &sa6->sin6_addr; > > +#endif > > > > #ifdef NI_NUMERICSCOPE > > if ((flags & NI_NUMERICSCOPE) != 0) { > > <DVM> Accepted. > > > > diff --git a/StdLib/BsdSocketLib/getsockopt.c > > b/StdLib/BsdSocketLib/getsockopt.c > > index 47b7c6f..fdbd6f7 100644 > > --- a/StdLib/BsdSocketLib/getsockopt.c > > +++ b/StdLib/BsdSocketLib/getsockopt.c > > @@ -60,6 +60,9 @@ getsockopt ( > > option_value, > > option_len, > > &errno ); > > + if (EFI_ERROR (Status)) { > > + errno = -1; > > + } > > } > > <DVM> Same comment as for bind(). pfnOptionGet() sets errno for all > > error conditions. It even sets errno to ESUCCESS, 0, if there are no > > errors. > > Please let me know if you are seeing getsockopt() failing but errno > is > > not set. > > > > // > > diff --git a/StdLib/BsdSocketLib/listen.c > > b/StdLib/BsdSocketLib/listen.c index 7c6d5f3..d3060a4 100644 > > --- a/StdLib/BsdSocketLib/listen.c > > +++ b/StdLib/BsdSocketLib/listen.c > > @@ -58,6 +58,9 @@ listen ( > > Status = pSocketProtocol->pfnListen ( pSocketProtocol, > > backlog, > > &errno ); > > + if (EFI_ERROR (Status)) { > > + errno = -1; > > + } > > } > > <DVM> Same comments as for bind() and getsockopt(). pfnListen() sets > > errno. > > > > // > > diff --git a/StdLib/BsdSocketLib/poll.c b/StdLib/BsdSocketLib/poll.c > > index dc17567..dcc367a 100644 > > --- a/StdLib/BsdSocketLib/poll.c > > +++ b/StdLib/BsdSocketLib/poll.c > > @@ -49,6 +49,9 @@ BslSocketPoll ( > > Events, > > &DetectedEvents, > > &errno ); > > + if (EFI_ERROR (Status)) { > > + errno = -1; > > + } > > } > > <DVM> Like bind(), getsockopt(), and listen(), the internal pfnPoll() > > function sets errno if an error occurs. The only errno value this > > implementation supports is EINVAL which indicates that a non-socket > or > > invalid fd was passed as the first argument. I noticed a bug in > > pfnPoll() where errno is not actually set to EINVAL. That will be > > fixed. > > > > // > > diff --git a/StdLib/BsdSocketLib/res_comp.c > > b/StdLib/BsdSocketLib/res_comp.c index cddda3e..7b5f2aa 100644 > > --- a/StdLib/BsdSocketLib/res_comp.c > > +++ b/StdLib/BsdSocketLib/res_comp.c > > @@ -168,7 +168,7 @@ res_hnok( > > const char *dn > > ) > > { > > - int ppch = '\0', pch = PERIOD, ch = *dn++; > > + int pch = PERIOD, ch = *dn++; > > > > while (ch != '\0') { > > int nch = *dn++; > > @@ -185,7 +185,7 @@ res_hnok( > > if (!middlechar(ch)) > > return (0); > > } > > - ppch = pch, pch = ch, ch = nch; > > + pch = ch, ch = nch; > > } > > return (1); > > } > > <DVM> Accepted. > > > > diff --git a/StdLib/BsdSocketLib/res_mkupdate.c > > b/StdLib/BsdSocketLib/res_mkupdate.c > > index 3201f31..1056fc6 100644 > > --- a/StdLib/BsdSocketLib/res_mkupdate.c > > +++ b/StdLib/BsdSocketLib/res_mkupdate.c > > @@ -100,7 +100,7 @@ int > > res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) { > > ns_updrec *rrecp_start = rrecp_in; > > HEADER *hp; > > - u_char *cp, *sp1, *sp2, *startp, *endp; > > + u_char *cp, *sp2, *startp, *endp; > > int n, i, soanum, multiline; > > ns_updrec *rrecp; > > struct in_addr ina; > > @@ -125,7 +125,6 @@ res_mkupdate(ns_updrec *rrecp_in, u_char *buf, > int > > buflen) { > > hp->id = htons(++_res.id); > > hp->opcode = ns_o_update; > > hp->rcode = NOERROR; > > - sp1 = buf + 2*INT16SZ; /* save pointer to zocount */ > > cp = buf + HFIXEDSZ; > > buflen -= HFIXEDSZ; > > dpp = dnptrs; > > <DVM> Accepted > > > > diff --git a/StdLib/BsdSocketLib/res_update.c > > b/StdLib/BsdSocketLib/res_update.c > > index a01d832..f3ff0ce 100644 > > --- a/StdLib/BsdSocketLib/res_update.c > > +++ b/StdLib/BsdSocketLib/res_update.c > > @@ -120,7 +120,6 @@ res_update(ns_updrec *rrecp_in) { > > int i, j, k = 0, n, ancount, nscount, arcount, rcode, rdatasize, > > newgroup, done, myzone, seen_before, numzones = 0; > > u_int16_t dlen, class, qclass, type, qtype; > > - u_int32_t ttl; > > > > if ((_res.options & RES_INIT) == 0 && res_init() == -1) { > > h_errno = NETDB_INTERNAL; > > @@ -302,7 +301,6 @@ res_update(ns_updrec *rrecp_in) { > > if (cp + INT32SZ + INT16SZ > eom) > > return (-1); > > /* continue processing the soa record */ > > - GETLONG(ttl, cp); > > GETSHORT(dlen, cp); > > if (cp + dlen > eom) > > return (-1); > > @@ -424,7 +422,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", > > return (-1); > > GETSHORT(type, cp); > > GETSHORT(class, cp); > > - GETLONG(ttl, cp); > > GETSHORT(dlen, cp); > > if (cp + dlen > eom) > > return (-1); > > @@ -450,7 +447,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", > > return (-1); > > GETSHORT(type, cp); > > GETSHORT(class, cp); > > - GETLONG(ttl, cp); > > GETSHORT(dlen, cp); > > if (cp + dlen > eom) > > return (-1); > > <DVM> Rejected. While ttl itself isn't used, the GETLONG(ttl, cp); > > invocations are required in order to advance the cp pointer over the > > ttl field of the header. > > > > diff --git a/StdLib/BsdSocketLib/setsockopt.c > > b/StdLib/BsdSocketLib/setsockopt.c > > index 64f3a35..7db7dea 100644 > > --- a/StdLib/BsdSocketLib/setsockopt.c > > +++ b/StdLib/BsdSocketLib/setsockopt.c > > @@ -59,6 +59,9 @@ setsockopt ( > > option_value, > > option_len, > > &errno ); > > + if (EFI_ERROR (Status)) { > > + errno = -1; > > + } > > } > > <DVM> Like bind(), etc. pfnOptionSet() sets errno if any error > occurs. > > > > // > > diff --git a/StdLib/EfiSocketLib/Ip4.c b/StdLib/EfiSocketLib/Ip4.c > > index ed71194..46425db 100644 > > --- a/StdLib/EfiSocketLib/Ip4.c > > +++ b/StdLib/EfiSocketLib/Ip4.c > > @@ -242,8 +242,6 @@ EslIp4OptionSet ( > > ) > > { > > BOOLEAN bTrueFalse; > > - socklen_t LengthInBytes; > > - UINT8 * pOptionData; > > EFI_STATUS Status; > > > > DBG_ENTER ( ); > > @@ -257,8 +255,6 @@ EslIp4OptionSet ( > > // > > // Determine if the option protocol matches > > // > > - LengthInBytes = 0; > > - pOptionData = NULL; > > switch ( OptionName ) { > > default: > > // > > @@ -283,12 +279,6 @@ EslIp4OptionSet ( > > bTrueFalse = FALSE; > > } > > pOptionValue = &bTrueFalse; > > - > > - // > > - // Set the option value > > - // > > - pOptionData = (UINT8 *)&pSocket->bIncludeHeader; > > - LengthInBytes = sizeof ( pSocket->bIncludeHeader ); > > } > > break; > > } > > <DVM> The patch is valid as far as removing two meaningless > variables. > > There is something else wrong with this function, though, since it > does > > nothing other than validate that OptionName is set to IP_HDRINCL. > > Query submitted to sub-package owner. > > > > @@ -653,7 +643,6 @@ EslIp4RxComplete ( > > ) > > { > > size_t LengthInBytes; > > - ESL_PORT * pPort; > > ESL_PACKET * pPacket; > > EFI_IP4_RECEIVE_DATA * pRxData; > > EFI_STATUS Status; > > @@ -663,7 +652,6 @@ EslIp4RxComplete ( > > // > > // Get the operation status. > > // > > - pPort = pIo->pPort; > > Status = pIo->Token.Ip4Rx.Status; > > > > // > > <DVM> Accepted change to EslIp4RxComplete(). > > > > diff --git a/StdLib/EfiSocketLib/Socket.c > > b/StdLib/EfiSocketLib/Socket.c index d53473e..740602d 100644 > > --- a/StdLib/EfiSocketLib/Socket.c > > +++ b/StdLib/EfiSocketLib/Socket.c > > @@ -4130,6 +4130,7 @@ EslSocketPortCloseComplete ( > > // > > Status = EslSocketPortCloseRxDone ( pPort ); > > DBG_EXIT_STATUS ( Status ); > > + ASSERT_EFI_ERROR (Status); > > } > > <DVM> Query submitted to the sub-package owner. The ASSERT may be > > superfluous. > > > > diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c > > index 7ece38d..0419ee2 100644 > > --- a/StdLib/EfiSocketLib/Tcp4.c > > +++ b/StdLib/EfiSocketLib/Tcp4.c > > @@ -840,7 +840,6 @@ EslTcp4ListenComplete ( > > EFI_HANDLE ChildHandle; > > struct sockaddr_in LocalAddress; > > EFI_TCP4_CONFIG_DATA * pConfigData; > > - ESL_LAYER * pLayer; > > ESL_PORT * pNewPort; > > ESL_SOCKET * pNewSocket; > > ESL_SOCKET * pSocket; > > @@ -869,7 +868,6 @@ EslTcp4ListenComplete ( > > // Allocate a socket for this connection > > // > > ChildHandle = NULL; > > - pLayer = &mEslLayer; > > Status = EslSocketAllocate ( &ChildHandle, > > DEBUG_CONNECTION, > > &pNewSocket ); > > <DVM> Accepted. > > > > @@ -1924,7 +1922,6 @@ EslTcp4TxBuffer ( > > ESL_PACKET ** ppQueueHead; > > ESL_PACKET ** ppQueueTail; > > ESL_PACKET * pPreviousPacket; > > - ESL_TCP4_CONTEXT * pTcp4; > > size_t * pTxBytes; > > EFI_TCP4_TRANSMIT_DATA * pTxData; > > EFI_STATUS Status; > > @@ -1951,7 +1948,6 @@ EslTcp4TxBuffer ( > > // > > // Determine the queue head > > // > > - pTcp4 = &pPort->Context.Tcp4; > > bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); > > bUrgentQueue = bUrgent > > && ( !pSocket->bOobInLine ) > > <DVM> Accepted. > > > > diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c > > index 21c4109..77a4a4c 100644 > > --- a/StdLib/EfiSocketLib/Tcp6.c > > +++ b/StdLib/EfiSocketLib/Tcp6.c > > @@ -871,7 +871,6 @@ EslTcp6ListenComplete ( > > EFI_HANDLE ChildHandle; > > struct sockaddr_in6 LocalAddress; > > EFI_TCP6_CONFIG_DATA * pConfigData; > > - ESL_LAYER * pLayer; > > ESL_PORT * pNewPort; > > ESL_SOCKET * pNewSocket; > > ESL_SOCKET * pSocket; > > @@ -900,7 +899,6 @@ EslTcp6ListenComplete ( > > // Allocate a socket for this connection > > // > > ChildHandle = NULL; > > - pLayer = &mEslLayer; > > Status = EslSocketAllocate ( &ChildHandle, > > DEBUG_CONNECTION, > > &pNewSocket ); > > <DVM> Accepted. > > > > @@ -1993,7 +1991,6 @@ EslTcp6TxBuffer ( > > ESL_PACKET ** ppQueueHead; > > ESL_PACKET ** ppQueueTail; > > ESL_PACKET * pPreviousPacket; > > - ESL_TCP6_CONTEXT * pTcp6; > > size_t * pTxBytes; > > EFI_TCP6_TRANSMIT_DATA * pTxData; > > EFI_STATUS Status; > > > > @@ -2020,7 +2017,6 @@ EslTcp6TxBuffer ( > > // > > // Determine the queue head > > // > > - pTcp6 = &pPort->Context.Tcp6; > > bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); > > bUrgentQueue = bUrgent > > && ( !pSocket->bOobInLine ) > > <DVM> Accepted. > > > > diff --git a/StdLib/LibC/Containers/Queues/Fifo.c > > b/StdLib/LibC/Containers/Queues/Fifo.c > > index 73254d2..93b62f8 100644 > > --- a/StdLib/LibC/Containers/Queues/Fifo.c > > +++ b/StdLib/LibC/Containers/Queues/Fifo.c > > @@ -278,7 +278,6 @@ FIFO_Dequeue ( > > { > > UINTN QPtr; > > UINT32 RDex; > > - UINT32 SizeOfElement; > > UINT32 i; > > > > assert(Self != NULL); > > @@ -289,7 +288,6 @@ FIFO_Dequeue ( > > } > > else { > > RDex = Self->ReadIndex; > > // Get this FIFO's Read Index > > - SizeOfElement = Self->ElementSize; > > // Get size of this FIFO's elements > > Count = MIN(Count, Self->Count(Self, AsElements)); > > // Lesser of requested or actual > > > > QPtr = (UINTN)Self->Queue + (RDex * Self->ElementSize); > > // Point to Read location in FIFO > > <DVM> Replacing all of the "Self->ElementSize" clauses in the > function > > with "SizeOfElement" would be more efficient. > > > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > > b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > > index 1c978ea..b498d2b 100644 > > --- a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c > > @@ -75,7 +75,7 @@ IIO_GetInChar ( > > { > > cIIO *This; > > cFIFO *InBuf; > > - EFI_STATUS Status; > > + size_t Status; > > ssize_t NumRead; > > wint_t RetVal; > > wchar_t InChar; > > @@ -92,8 +92,10 @@ IIO_GetInChar ( > > } > > if(BufCnt > 0) { > > Status = InBuf->Read(InBuf, &InChar, 1); > > - --BufCnt; > > - NumRead = 1; > > + if (Status > 0) { > > + --BufCnt; > > + NumRead = 1; > > + } > > } > > else { > > NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, > > sizeof(wchar_t), &InChar); @@ -104,7 +106,7 @@ IIO_GetInChar ( > > else { > > RetVal = (wint_t)InChar; > > } > > - return InChar; > > + return RetVal; > > } > > > > /** Get the current cursor position. > > <DVM> Accepted. > > > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > > b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > > index 927f4f4..2c5e81d 100644 > > --- a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c > > @@ -63,7 +63,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > > wchar_t InCh) > > UINT32 CurRow; // Current cursor row on the > > screen > > UINT32 PrevColumn; // Previous column. Used to > > detect wrapping. > > UINT32 AdjColumn; // Current cursor column on the > > screen > > - UINT32 AdjRow; // Current cursor row on the > > screen > > > > RetVal = -1; > > wcb = wc; > > @@ -79,7 +78,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > > wchar_t InCh) > > CurRow = This->CurrentXY.Row; > > > > numW = 1; // The majority of characters buffer one > > character > > - AdjRow = 0; // Most characters just cause horizontal > > movement > > AdjColumn = 0; > > if(OFlag & OPOST) { > > /* Perform output processing */ > > @@ -127,7 +125,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, > > wchar_t InCh) > > numW = 2; > > CurColumn = 0; > > } > > - AdjRow = 1; > > break; //}} > > > > case CHAR_BACKSPACE: //{{ > > <DVM> Accepted. > > > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > > b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > > index bcae9c8..3b226d1 100644 > > --- a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > > +++ b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c > > @@ -37,7 +37,6 @@ IIO_NonCanonRead ( > > cIIO *This; > > cFIFO *InBuf; > > struct termios *Termio; > > - EFI_STATUS Status; > > ssize_t NumRead; > > cc_t tioMin; > > cc_t tioTime; > > @@ -74,7 +73,7 @@ IIO_NonCanonRead ( > > if(InBuf->IsEmpty(InBuf)) { > > NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, > > sizeof(wchar_t), &InChar); > > if(NumRead > 0) { > > - Status = InBuf->Write(InBuf, &InChar, 1); // Buffer the > > character > > + InBuf->Write(InBuf, &InChar, 1); // Buffer the character > > } > > } > > // break; > > <DVM> Accepted. > > > > diff --git a/StdLib/LibC/Uefi/writev.c b/StdLib/LibC/Uefi/writev.c > > index 9cff086..56712c5 100644 > > --- a/StdLib/LibC/Uefi/writev.c > > +++ b/StdLib/LibC/Uefi/writev.c > > @@ -101,7 +101,7 @@ writev( > > ) > > { > > const struct iovec *pVecTmp; > > - char *pBuf, *pBufTmp; > > + char *pBuf; > > size_t TotalBytes, i, ret; > > > > // > > @@ -126,7 +126,7 @@ writev( > > // Copy vectors to the buffer > > // > > > > - for (pBufTmp = pBuf; iovcnt; iovcnt--) { > > + for (; iovcnt; iovcnt--) { > > bcopy(iov->iov_base, pBuf, iov->iov_len); > > pBuf += iov->iov_len; > > iov++; > > <DVM> Accepted. > > -- > > 2.1.1 > > > > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 ------------------------------------------------------------------------------
diff --git a/StdLib/BsdSocketLib/bind.c b/StdLib/BsdSocketLib/bind.c index f544c94..f22fe2f 100644 --- a/StdLib/BsdSocketLib/bind.c +++ b/StdLib/BsdSocketLib/bind.c @@ -65,6 +65,9 @@ bind ( name, namelen, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } } // diff --git a/StdLib/BsdSocketLib/getnameinfo.c b/StdLib/BsdSocketLib/getnameinfo.c index fab3460..0318d93 100644 --- a/StdLib/BsdSocketLib/getnameinfo.c +++ b/StdLib/BsdSocketLib/getnameinfo.c @@ -411,15 +411,19 @@ ip6_sa2str( int flags ) { - unsigned int ifindex; +#if 0 + unsigned int ifindex; const struct in6_addr *a6; +#endif int n; _DIAGASSERT(sa6 != NULL); _DIAGASSERT(buf != NULL); - ifindex = (unsigned int)sa6->sin6_scope_id; +#if 0 + ifindex = (unsigned int)sa6->sin6_scope_id; a6 = &sa6->sin6_addr; +#endif #ifdef NI_NUMERICSCOPE if ((flags & NI_NUMERICSCOPE) != 0) { diff --git a/StdLib/BsdSocketLib/getsockopt.c b/StdLib/BsdSocketLib/getsockopt.c index 47b7c6f..fdbd6f7 100644 --- a/StdLib/BsdSocketLib/getsockopt.c +++ b/StdLib/BsdSocketLib/getsockopt.c @@ -60,6 +60,9 @@ getsockopt ( option_value, option_len, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } } // diff --git a/StdLib/BsdSocketLib/listen.c b/StdLib/BsdSocketLib/listen.c index 7c6d5f3..d3060a4 100644 --- a/StdLib/BsdSocketLib/listen.c +++ b/StdLib/BsdSocketLib/listen.c @@ -58,6 +58,9 @@ listen ( Status = pSocketProtocol->pfnListen ( pSocketProtocol, backlog, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } } // diff --git a/StdLib/BsdSocketLib/poll.c b/StdLib/BsdSocketLib/poll.c index dc17567..dcc367a 100644 --- a/StdLib/BsdSocketLib/poll.c +++ b/StdLib/BsdSocketLib/poll.c @@ -49,6 +49,9 @@ BslSocketPoll ( Events, &DetectedEvents, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } } // diff --git a/StdLib/BsdSocketLib/res_comp.c b/StdLib/BsdSocketLib/res_comp.c index cddda3e..7b5f2aa 100644 --- a/StdLib/BsdSocketLib/res_comp.c +++ b/StdLib/BsdSocketLib/res_comp.c @@ -168,7 +168,7 @@ res_hnok( const char *dn ) { - int ppch = '\0', pch = PERIOD, ch = *dn++; + int pch = PERIOD, ch = *dn++; while (ch != '\0') { int nch = *dn++; @@ -185,7 +185,7 @@ res_hnok( if (!middlechar(ch)) return (0); } - ppch = pch, pch = ch, ch = nch; + pch = ch, ch = nch; } return (1); } diff --git a/StdLib/BsdSocketLib/res_mkupdate.c b/StdLib/BsdSocketLib/res_mkupdate.c index 3201f31..1056fc6 100644 --- a/StdLib/BsdSocketLib/res_mkupdate.c +++ b/StdLib/BsdSocketLib/res_mkupdate.c @@ -100,7 +100,7 @@ int res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) { ns_updrec *rrecp_start = rrecp_in; HEADER *hp; - u_char *cp, *sp1, *sp2, *startp, *endp; + u_char *cp, *sp2, *startp, *endp; int n, i, soanum, multiline; ns_updrec *rrecp; struct in_addr ina; @@ -125,7 +125,6 @@ res_mkupdate(ns_updrec *rrecp_in, u_char *buf, int buflen) { hp->id = htons(++_res.id); hp->opcode = ns_o_update; hp->rcode = NOERROR; - sp1 = buf + 2*INT16SZ; /* save pointer to zocount */ cp = buf + HFIXEDSZ; buflen -= HFIXEDSZ; dpp = dnptrs; diff --git a/StdLib/BsdSocketLib/res_update.c b/StdLib/BsdSocketLib/res_update.c index a01d832..f3ff0ce 100644 --- a/StdLib/BsdSocketLib/res_update.c +++ b/StdLib/BsdSocketLib/res_update.c @@ -120,7 +120,6 @@ res_update(ns_updrec *rrecp_in) { int i, j, k = 0, n, ancount, nscount, arcount, rcode, rdatasize, newgroup, done, myzone, seen_before, numzones = 0; u_int16_t dlen, class, qclass, type, qtype; - u_int32_t ttl; if ((_res.options & RES_INIT) == 0 && res_init() == -1) { h_errno = NETDB_INTERNAL; @@ -302,7 +301,6 @@ res_update(ns_updrec *rrecp_in) { if (cp + INT32SZ + INT16SZ > eom) return (-1); /* continue processing the soa record */ - GETLONG(ttl, cp); GETSHORT(dlen, cp); if (cp + dlen > eom) return (-1); @@ -424,7 +422,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", return (-1); GETSHORT(type, cp); GETSHORT(class, cp); - GETLONG(ttl, cp); GETSHORT(dlen, cp); if (cp + dlen > eom) return (-1); @@ -450,7 +447,6 @@ ans=%d, auth=%d, add=%d, rcode=%d\n", return (-1); GETSHORT(type, cp); GETSHORT(class, cp); - GETLONG(ttl, cp); GETSHORT(dlen, cp); if (cp + dlen > eom) return (-1); diff --git a/StdLib/BsdSocketLib/setsockopt.c b/StdLib/BsdSocketLib/setsockopt.c index 64f3a35..7db7dea 100644 --- a/StdLib/BsdSocketLib/setsockopt.c +++ b/StdLib/BsdSocketLib/setsockopt.c @@ -59,6 +59,9 @@ setsockopt ( option_value, option_len, &errno ); + if (EFI_ERROR (Status)) { + errno = -1; + } } // diff --git a/StdLib/EfiSocketLib/Ip4.c b/StdLib/EfiSocketLib/Ip4.c index ed71194..46425db 100644 --- a/StdLib/EfiSocketLib/Ip4.c +++ b/StdLib/EfiSocketLib/Ip4.c @@ -242,8 +242,6 @@ EslIp4OptionSet ( ) { BOOLEAN bTrueFalse; - socklen_t LengthInBytes; - UINT8 * pOptionData; EFI_STATUS Status; DBG_ENTER ( ); @@ -257,8 +255,6 @@ EslIp4OptionSet ( // // Determine if the option protocol matches // - LengthInBytes = 0; - pOptionData = NULL; switch ( OptionName ) { default: // @@ -283,12 +279,6 @@ EslIp4OptionSet ( bTrueFalse = FALSE; } pOptionValue = &bTrueFalse; - - // - // Set the option value - // - pOptionData = (UINT8 *)&pSocket->bIncludeHeader; - LengthInBytes = sizeof ( pSocket->bIncludeHeader ); } break; } @@ -653,7 +643,6 @@ EslIp4RxComplete ( ) { size_t LengthInBytes; - ESL_PORT * pPort; ESL_PACKET * pPacket; EFI_IP4_RECEIVE_DATA * pRxData; EFI_STATUS Status; @@ -663,7 +652,6 @@ EslIp4RxComplete ( // // Get the operation status. // - pPort = pIo->pPort; Status = pIo->Token.Ip4Rx.Status; // diff --git a/StdLib/EfiSocketLib/Socket.c b/StdLib/EfiSocketLib/Socket.c index d53473e..740602d 100644 --- a/StdLib/EfiSocketLib/Socket.c +++ b/StdLib/EfiSocketLib/Socket.c @@ -4130,6 +4130,7 @@ EslSocketPortCloseComplete ( // Status = EslSocketPortCloseRxDone ( pPort ); DBG_EXIT_STATUS ( Status ); + ASSERT_EFI_ERROR (Status); } diff --git a/StdLib/EfiSocketLib/Tcp4.c b/StdLib/EfiSocketLib/Tcp4.c index 7ece38d..0419ee2 100644 --- a/StdLib/EfiSocketLib/Tcp4.c +++ b/StdLib/EfiSocketLib/Tcp4.c @@ -840,7 +840,6 @@ EslTcp4ListenComplete ( EFI_HANDLE ChildHandle; struct sockaddr_in LocalAddress; EFI_TCP4_CONFIG_DATA * pConfigData; - ESL_LAYER * pLayer; ESL_PORT * pNewPort; ESL_SOCKET * pNewSocket; ESL_SOCKET * pSocket; @@ -869,7 +868,6 @@ EslTcp4ListenComplete ( // Allocate a socket for this connection // ChildHandle = NULL; - pLayer = &mEslLayer; Status = EslSocketAllocate ( &ChildHandle, DEBUG_CONNECTION, &pNewSocket ); @@ -1924,7 +1922,6 @@ EslTcp4TxBuffer ( ESL_PACKET ** ppQueueHead; ESL_PACKET ** ppQueueTail; ESL_PACKET * pPreviousPacket; - ESL_TCP4_CONTEXT * pTcp4; size_t * pTxBytes; EFI_TCP4_TRANSMIT_DATA * pTxData; EFI_STATUS Status; @@ -1951,7 +1948,6 @@ EslTcp4TxBuffer ( // // Determine the queue head // - pTcp4 = &pPort->Context.Tcp4; bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); bUrgentQueue = bUrgent && ( !pSocket->bOobInLine ) diff --git a/StdLib/EfiSocketLib/Tcp6.c b/StdLib/EfiSocketLib/Tcp6.c index 21c4109..77a4a4c 100644 --- a/StdLib/EfiSocketLib/Tcp6.c +++ b/StdLib/EfiSocketLib/Tcp6.c @@ -871,7 +871,6 @@ EslTcp6ListenComplete ( EFI_HANDLE ChildHandle; struct sockaddr_in6 LocalAddress; EFI_TCP6_CONFIG_DATA * pConfigData; - ESL_LAYER * pLayer; ESL_PORT * pNewPort; ESL_SOCKET * pNewSocket; ESL_SOCKET * pSocket; @@ -900,7 +899,6 @@ EslTcp6ListenComplete ( // Allocate a socket for this connection // ChildHandle = NULL; - pLayer = &mEslLayer; Status = EslSocketAllocate ( &ChildHandle, DEBUG_CONNECTION, &pNewSocket ); @@ -1993,7 +1991,6 @@ EslTcp6TxBuffer ( ESL_PACKET ** ppQueueHead; ESL_PACKET ** ppQueueTail; ESL_PACKET * pPreviousPacket; - ESL_TCP6_CONTEXT * pTcp6; size_t * pTxBytes; EFI_TCP6_TRANSMIT_DATA * pTxData; EFI_STATUS Status; @@ -2020,7 +2017,6 @@ EslTcp6TxBuffer ( // // Determine the queue head // - pTcp6 = &pPort->Context.Tcp6; bUrgent = (BOOLEAN)( 0 != ( Flags & MSG_OOB )); bUrgentQueue = bUrgent && ( !pSocket->bOobInLine ) diff --git a/StdLib/LibC/Containers/Queues/Fifo.c b/StdLib/LibC/Containers/Queues/Fifo.c index 73254d2..93b62f8 100644 --- a/StdLib/LibC/Containers/Queues/Fifo.c +++ b/StdLib/LibC/Containers/Queues/Fifo.c @@ -278,7 +278,6 @@ FIFO_Dequeue ( { UINTN QPtr; UINT32 RDex; - UINT32 SizeOfElement; UINT32 i; assert(Self != NULL); @@ -289,7 +288,6 @@ FIFO_Dequeue ( } else { RDex = Self->ReadIndex; // Get this FIFO's Read Index - SizeOfElement = Self->ElementSize; // Get size of this FIFO's elements Count = MIN(Count, Self->Count(Self, AsElements)); // Lesser of requested or actual QPtr = (UINTN)Self->Queue + (RDex * Self->ElementSize); // Point to Read location in FIFO diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c index 1c978ea..b498d2b 100644 --- a/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c @@ -75,7 +75,7 @@ IIO_GetInChar ( { cIIO *This; cFIFO *InBuf; - EFI_STATUS Status; + size_t Status; ssize_t NumRead; wint_t RetVal; wchar_t InChar; @@ -92,8 +92,10 @@ IIO_GetInChar ( } if(BufCnt > 0) { Status = InBuf->Read(InBuf, &InChar, 1); - --BufCnt; - NumRead = 1; + if (Status > 0) { + --BufCnt; + NumRead = 1; + } } else { NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, sizeof(wchar_t), &InChar); @@ -104,7 +106,7 @@ IIO_GetInChar ( else { RetVal = (wint_t)InChar; } - return InChar; + return RetVal; } /** Get the current cursor position. diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c index 927f4f4..2c5e81d 100644 --- a/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c +++ b/StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c @@ -63,7 +63,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, wchar_t InCh) UINT32 CurRow; // Current cursor row on the screen UINT32 PrevColumn; // Previous column. Used to detect wrapping. UINT32 AdjColumn; // Current cursor column on the screen - UINT32 AdjRow; // Current cursor row on the screen RetVal = -1; wcb = wc; @@ -79,7 +78,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, wchar_t InCh) CurRow = This->CurrentXY.Row; numW = 1; // The majority of characters buffer one character - AdjRow = 0; // Most characters just cause horizontal movement AdjColumn = 0; if(OFlag & OPOST) { /* Perform output processing */ @@ -127,7 +125,6 @@ IIO_WriteOne(struct __filedes *filp, cFIFO *OBuf, wchar_t InCh) numW = 2; CurColumn = 0; } - AdjRow = 1; break; //}} case CHAR_BACKSPACE: //{{ diff --git a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c index bcae9c8..3b226d1 100644 --- a/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c +++ b/StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c @@ -37,7 +37,6 @@ IIO_NonCanonRead ( cIIO *This; cFIFO *InBuf; struct termios *Termio; - EFI_STATUS Status; ssize_t NumRead; cc_t tioMin; cc_t tioTime; @@ -74,7 +73,7 @@ IIO_NonCanonRead ( if(InBuf->IsEmpty(InBuf)) { NumRead = filp->f_ops->fo_read(filp, &filp->f_offset, sizeof(wchar_t), &InChar); if(NumRead > 0) { - Status = InBuf->Write(InBuf, &InChar, 1); // Buffer the character + InBuf->Write(InBuf, &InChar, 1); // Buffer the character } } // break; diff --git a/StdLib/LibC/Uefi/writev.c b/StdLib/LibC/Uefi/writev.c index 9cff086..56712c5 100644 --- a/StdLib/LibC/Uefi/writev.c +++ b/StdLib/LibC/Uefi/writev.c @@ -101,7 +101,7 @@ writev( ) { const struct iovec *pVecTmp; - char *pBuf, *pBufTmp; + char *pBuf; size_t TotalBytes, i, ret; // @@ -126,7 +126,7 @@ writev( // Copy vectors to the buffer // - for (pBufTmp = pBuf; iovcnt; iovcnt--) { + for (; iovcnt; iovcnt--) { bcopy(iov->iov_base, pBuf, iov->iov_len); pBuf += iov->iov_len; iov++;
Some of the variables are unused that trigger a GCC warning/error. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <olivier.martin@arm.com> --- StdLib/BsdSocketLib/bind.c | 3 +++ StdLib/BsdSocketLib/getnameinfo.c | 8 ++++++-- StdLib/BsdSocketLib/getsockopt.c | 3 +++ StdLib/BsdSocketLib/listen.c | 3 +++ StdLib/BsdSocketLib/poll.c | 3 +++ StdLib/BsdSocketLib/res_comp.c | 4 ++-- StdLib/BsdSocketLib/res_mkupdate.c | 3 +-- StdLib/BsdSocketLib/res_update.c | 4 ---- StdLib/BsdSocketLib/setsockopt.c | 3 +++ StdLib/EfiSocketLib/Ip4.c | 12 ------------ StdLib/EfiSocketLib/Socket.c | 1 + StdLib/EfiSocketLib/Tcp4.c | 4 ---- StdLib/EfiSocketLib/Tcp6.c | 4 ---- StdLib/LibC/Containers/Queues/Fifo.c | 2 -- StdLib/LibC/Uefi/InteractiveIO/IIOutilities.c | 10 ++++++---- StdLib/LibC/Uefi/InteractiveIO/IIOwrite.c | 3 --- StdLib/LibC/Uefi/InteractiveIO/NonCanonRead.c | 3 +-- StdLib/LibC/Uefi/writev.c | 4 ++-- 18 files changed, 34 insertions(+), 43 deletions(-)