From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 6 Mar 2023 18:44:38 +0000 (+0000)
Subject: Reduce stale errno usage (#1302)
X-Git-Tag: SQUID_6_9~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6ec0a25cbe8c42b291ef7a0096b488478b83e68f;p=thirdparty%2Fsquid.git

Reduce stale errno usage (#1302)

This covers a few easy cases, one of which resulted in wasted triage
time. More changes are needed to cover all potentially stale errno uses.
---

diff --git a/src/comm.cc b/src/comm.cc
index 2eefef801b..f6803701a0 100644
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -378,7 +378,7 @@ comm_openex(int sock_type,
     debugs(50, 3, "comm_openex: Attempt open socket for: " << addr );
 
     new_socket = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol);
-    int xerrno = errno;
+    const auto firstErrNo = errno;
 
     /* under IPv6 there is the possibility IPv6 is present but disabled. */
     /* try again as IPv4-native if possible */
@@ -391,6 +391,8 @@ comm_openex(int sock_type,
         AI->ai_protocol = proto;
         debugs(50, 3, "Attempt fallback open socket for: " << addr );
         new_socket = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol);
+        // TODO: Report failures of this second socket() call.
+        // if both socket() calls fail, we use firstErrNo
         debugs(50, 2, "attempt open " << note << " socket on: " << addr);
     }
 
@@ -399,16 +401,16 @@ comm_openex(int sock_type,
          * are failing because the open file table is full.  This
          * limits the number of simultaneous clients */
 
-        if (limitError(errno)) {
-            debugs(50, DBG_IMPORTANT, MYNAME << "socket failure: " << xstrerr(xerrno));
+        if (limitError(firstErrNo)) {
+            debugs(50, DBG_IMPORTANT, MYNAME << "socket failure: " << xstrerr(firstErrNo));
             fdAdjustReserved();
         } else {
-            debugs(50, DBG_CRITICAL, MYNAME << "socket failure: " << xstrerr(xerrno));
+            debugs(50, DBG_CRITICAL, MYNAME << "socket failure: " << xstrerr(firstErrNo));
         }
 
         Ip::Address::FreeAddr(AI);
 
-        errno = xerrno; // restore for caller
+        errno = firstErrNo; // restore for caller
         return -1;
     }
 
@@ -434,7 +436,9 @@ comm_openex(int sock_type,
 
     // XXX transition only. prevent conn from closing the new FD on function exit.
     conn->fd = -1;
-    errno = xerrno; // restore for caller
+    // XXX: firstErrNo is not applicable here -- socket() calls succeeded above!
+    // TODO: Stop reporting error codes via errno.
+    errno = firstErrNo;
     return new_socket;
 }
 
@@ -890,11 +894,11 @@ _comm_close(int fd, char const *file, int line)
     // notify read/write handlers after canceling select reservations, if any
     if (COMMIO_FD_WRITECB(fd)->active()) {
         Comm::SetSelect(fd, COMM_SELECT_WRITE, nullptr, nullptr, 0);
-        COMMIO_FD_WRITECB(fd)->finish(Comm::ERR_CLOSING, errno);
+        COMMIO_FD_WRITECB(fd)->finish(Comm::ERR_CLOSING, 0);
     }
     if (COMMIO_FD_READCB(fd)->active()) {
         Comm::SetSelect(fd, COMM_SELECT_READ, nullptr, nullptr, 0);
-        COMMIO_FD_READCB(fd)->finish(Comm::ERR_CLOSING, errno);
+        COMMIO_FD_READCB(fd)->finish(Comm::ERR_CLOSING, 0);
     }
 
 #if USE_DELAY_POOLS
diff --git a/src/fs_io.cc b/src/fs_io.cc
index eaf0dc1a42..2309a9975b 100644
--- a/src/fs_io.cc
+++ b/src/fs_io.cc
@@ -211,6 +211,7 @@ diskHandleWrite(int fd, void *)
     len = FD_WRITE_METHOD(fd,
                           fdd->write_q->buf + fdd->write_q->buf_offset,
                           fdd->write_q->len - fdd->write_q->buf_offset);
+    const auto xerrno = errno;
 
     debugs(6, 3, "diskHandleWrite: FD " << fd << " len = " << len);
 
@@ -219,9 +220,8 @@ diskHandleWrite(int fd, void *)
     fd_bytes(fd, len, FD_WRITE);
 
     if (len < 0) {
-        if (!ignoreErrno(errno)) {
-            status = errno == ENOSPC ? DISK_NO_SPACE_LEFT : DISK_ERROR;
-            int xerrno = errno;
+        if (!ignoreErrno(xerrno)) {
+            status = xerrno == ENOSPC ? DISK_NO_SPACE_LEFT : DISK_ERROR;
             debugs(50, DBG_IMPORTANT, "ERROR: diskHandleWrite: FD " << fd << ": disk write failure: " << xstrerr(xerrno));
 
             /*
@@ -494,7 +494,7 @@ FileRename(const SBuf &from, const SBuf &to)
         return true;
 
     int xerrno = errno;
-    debugs(21, (errno == ENOENT ? 2 : DBG_IMPORTANT), "ERROR: Cannot rename " << from << " to " << to << ": " << xstrerr(xerrno));
+    debugs(21, (xerrno == ENOENT ? 2 : DBG_IMPORTANT), "ERROR: Cannot rename " << from << " to " << to << ": " << xstrerr(xerrno));
 
     return false;
 }