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.mcdan...@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.mcdan...@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.mar...@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.mar...@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 > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel