Make comm_close a method of Comm::Connection

diff --git a/src/comm.cc b/src/comm.cc
index 18ca3ae6349d98d520e3696bbf86370796fc2d4b..d7bf01d583914cdb180a266c6bdcfbfdfcb43d3d 100644 (file)
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -1149,16 +1149,6 @@ comm_close_complete(int fd, void *data)
     Comm::AcceptLimiter::Instance().kick();
 }
 
-/*
- * Close the socket fd in use by a connection.
- */
-void
-_comm_close(Comm::ConnectionPointer conn, char const *file, int line)
-{
-    _comm_close(conn->fd, file, line);
-    conn->fd = -1;
-}
-
 /*
  * Close the socket fd.
  *

diff --git a/src/comm.h b/src/comm.h
index f7f9a390f0447cebc18411eaa7214db673f1e55f..dd4e6ccaa5adef648ad913f641a1a222e85be619 100644 (file)
--- a/src/comm.h
+++ b/src/comm.h
@@ -22,14 +22,11 @@ SQUIDCEXTERN int commUnsetNonBlocking(int fd);
 SQUIDCEXTERN void commSetCloseOnExec(int fd);
 SQUIDCEXTERN void commSetTcpKeepalive(int fd, int idle, int interval, int timeout);
 extern void _comm_close(int fd, char const *file, int line);
-extern void _comm_close(Comm::ConnectionPointer conn, char const *file, int line);
 #define comm_close(x) (_comm_close((x), __FILE__, __LINE__))
 SQUIDCEXTERN void comm_reset_close(int fd);
 #if LINGERING_CLOSE
 SQUIDCEXTERN void comm_lingering_close(int fd);
 #endif
-SQUIDCEXTERN void commConnectStart(int fd, const char *, u_short, CNCB *, void *);
-void commConnectStart(int fd, const char *, u_short, AsyncCall::Pointer &cb);
 
 SQUIDCEXTERN int comm_connect_addr(int sock, const Ip::Address &addr);
 SQUIDCEXTERN void comm_init(void);

diff --git a/src/comm/Connection.cc b/src/comm/Connection.cc
index 951dc519c59daa8c695123c4141a160bb903c6ae..3d6e4ec48596c8e902dd8ccf95f7442a183a78a3 100644 (file)
--- a/src/comm/Connection.cc
+++ b/src/comm/Connection.cc
@@ -37,14 +37,20 @@ Comm::Connection::operator =(const Comm::Connection &c)
 
 Comm::Connection::~Connection()
 {
-    if (fd >= 0) {
-        comm_close(fd);
-    }
+    close();
     if (_peer) {
         cbdataReferenceDone(_peer);
     }
 }
 
+void
+Comm::Connection::close()
+{
+    if (isOpen())
+        comm_close(fd);
+    fd = -1;
+}
+
 void
 Comm::Connection::setPeer(peer *p)
 {

diff --git a/src/comm/Connection.h b/src/comm/Connection.h
index 9c325dc421d8e14bd96f30e04e7e79f233756fd3..76da31315aa3f9517cc6abf0d871e476e4a3be92 100644 (file)
--- a/src/comm/Connection.h
+++ b/src/comm/Connection.h
@@ -61,8 +61,10 @@ namespace Comm {
  * object for state data. But a semantic equivalent for FD with easily
  * accessible cached properties not requiring repeated complex lookups.
  *
- * While the properties may be changed, they should be considered read-only
- * outside of the Comm layer code.
+ * While the properties may be changed, this is for teh purpose of creating
+ * potential connection descriptors which may be opened. Properties should
+ * be considered read-only outside of the Comm layer code once the connection
+ * is open.
  *
  * These objects must not be passed around directly,
  * but a Comm::ConnectionPointer must be passed instead.
@@ -84,6 +86,11 @@ public:
     /** see Comm::Connection::Connection */
     const Connection & operator =(const Connection &c);
 
+    void close();
+
+    /** determine whether this object describes an active connection or not. */
+    bool isOpen() { return (fd >= 0); }
+
     /** Address/Port for the Squid end of a TCP link. */
     Ip::Address local;
 

diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index dfe7803c29eddf605b071bbf5110086dc0a1dce1..f495cd99f87d9ab2441693361f2d86eb7a85f4db 100644 (file)
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -1,4 +1,3 @@
-
 /*
  * $Id$
  *
@@ -35,6 +34,7 @@
 
 #include "squid.h"
 #include "CacheManager.h"
+#include "comm/Connection.h"
 #include "comm/ConnectStateData.h"
 #include "comm.h"
 #include "event.h"

diff --git a/src/forward.cc b/src/forward.cc
index 4fcda73b68972d3b6313df42168e1dd2da74c57a..4070724700b4657a31f6738883be6966327f3588 100644 (file)
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -80,10 +80,7 @@ FwdState::abort(void* d)
     FwdState* fwd = (FwdState*)d;
     Pointer tmp = fwd; // Grab a temporary pointer to keep the object alive during our scope.
 
-    if (fwd->paths[0]->fd >= 0) {
-        comm_close(fwd->paths[0]);
-    }
-
+    fwd->paths[0]->close();
     fwd->self = NULL;
 }
 
@@ -173,7 +170,7 @@ FwdState::~FwdState()
     if (paths[0]->fd > -1) {
         comm_remove_close_handler(paths[0]->fd, fwdServerClosedWrapper, this);
         debugs(17, 3, HERE << "closing FD " << paths[0]->fd);
-        comm_close(paths[0]);
+        paths[0]->close();
     }
 
     paths.clean();
@@ -576,7 +573,7 @@ FwdState::negotiateSSL(int fd)
                 paths[0]->getPeer()->stats.conn_open--;
             }
 
-            comm_close(paths[0]);
+            paths[0]->close();
             return;
         }
     }
@@ -672,7 +669,7 @@ FwdState::connectDone(Comm::ConnectionPointer conn, Comm::PathsPointer result_pa
             if (paths[0]->getPeer())
                 peerConnectFailed(paths[0]->getPeer());
 
-            comm_close(paths[0]);
+            paths[0]->close();
         }
 
         return;
@@ -726,7 +723,7 @@ FwdState::connectTimeout(int fd)
                 peerConnectFailed(paths[0]->getPeer());
     }
 
-    comm_close(paths[0]);
+    paths[0]->close();
 }
 
 /**
@@ -764,7 +761,7 @@ FwdState::connectStart()
         ConnStateData *pinned_connection = request->pinnedConnection();
         assert(pinned_connection);
         conn->fd = pinned_connection->validatePinnedConnection(request, conn->getPeer());
-        if (conn->fd >= 0) {
+        if (conn->isOpen()) {
             pinned_connection->unpinConnection();
 #if 0
             if (!conn->getPeer())
@@ -806,7 +803,7 @@ FwdState::connectStart()
     }
     conn->remote.SetPort(port);
 
-    if (conn->fd >= 0) {
+    if (conn->isOpen()) {
         debugs(17, 3, HERE << "reusing pconn FD " << conn->fd);
         n_tries++;
 
@@ -959,7 +956,7 @@ FwdState::dispatch()
              * transient (network) error; its a bug.
              */
             flags.dont_retry = 1;
-            comm_close(paths[0]);
+            paths[0]->close();
             break;
         }
     }

diff --git a/src/ftp.cc b/src/ftp.cc
index edda02653af75a41859f39edcf634158a7b52afb..9f3406a12ccd2deb4cd9495c072d65d6d1706dbb 100644 (file)
--- a/src/ftp.cc
+++ b/src/ftp.cc
@@ -2619,7 +2619,6 @@ ftpReadPasv(FtpStateData * ftpState)
     int n;
     u_short port;
     Ip::Address ipa_remote;
-    int fd = ftpState->data.fd;
     char *buf;
     LOCAL_ARRAY(char, ipaddr, 1024);
     debugs(9, 3, HERE);
@@ -2964,7 +2963,8 @@ void FtpStateData::ftpAcceptDataConnection(const CommAcceptCbParams &io)
                    fd_table[ctrl.fd].ipaddr);
 
             /* close the bad sources connection down ASAP. */
-            comm_close(io.details);
+            Comm::ConnectionPointer nonConst = io.details;
+            nonConst->close();
 
             /* we are ony accepting once, so need to re-open the listener socket. */
             typedef CommCbMemFunT<FtpStateData, CommAcceptCbParams> acceptDialer;

diff --git a/src/gopher.cc b/src/gopher.cc
index 3e7b257ce08069a778b8aec900e17315525ec996..35d695fc1060d1968a25d64cc1c19bb3dfa30641 100644 (file)
--- a/src/gopher.cc
+++ b/src/gopher.cc
@@ -1031,7 +1031,7 @@ gopherStart(FwdState * fwd)
 
         gopherToHTML(gopherState, (char *) NULL, 0);
         fwd->complete();
-        comm_close(fwd->conn());
+        fwd->conn()->close();
         return;
     }
 

diff --git a/src/ident/Ident.cc b/src/ident/Ident.cc
index 5fdf12159a869f7334e927b11d49381891ce0ba7..65c038bbd4f5e7f8aa7e6dabbbdea4861b6f4d4c 100644 (file)
--- a/src/ident/Ident.cc
+++ b/src/ident/Ident.cc
@@ -37,6 +37,7 @@
 #if USE_IDENT
 
 #include "comm.h"
+#include "comm/Connection.h"
 #include "comm/ConnectStateData.h"
 #include "CommCalls.h"
 #include "ident/Config.h"
@@ -102,7 +103,7 @@ Ident::Close(int fdnotused, void *data)
 {
     IdentStateData *state = (IdentStateData *)data;
     identCallback(state, NULL);
-    comm_close(&(state->conn));
+    state->conn.close();
     hash_remove_link(ident_hash, (hash_link *) state);
     xfree(state->hash.key);
     cbdataFree(state);
@@ -113,7 +114,7 @@ Ident::Timeout(int fd, void *data)
 {
     IdentStateData *state = (IdentStateData *)data;
     debugs(30, 3, HERE << "FD " << fd << ", " << state->conn.remote);
-    comm_close(&(state->conn));
+    state->conn.close();
 }
 
 void
@@ -141,7 +142,7 @@ Ident::ConnectDone(Comm::ConnectionPointer conn, Comm::PathsPointer unused, comm
 
     if (c == NULL) {
         /* no clients care */
-        comm_close(conn);
+        conn->close();
         return;
     }
 
@@ -168,7 +169,7 @@ Ident::ReadReply(int fd, char *buf, size_t len, comm_err_t flag, int xerrno, voi
     assert(fd == state->conn.fd);
 
     if (flag != COMM_OK || len <= 0) {
-        comm_close(&(state->conn));
+        state->conn.close();
         return;
     }
 
@@ -194,7 +195,7 @@ Ident::ReadReply(int fd, char *buf, size_t len, comm_err_t flag, int xerrno, voi
         }
     }
 
-    comm_close(&(state->conn));
+    state->conn.close();
 }
 
 void

diff --git a/src/ipcache.cc b/src/ipcache.cc
index d69f41426ddd29cae26cbd9d17fa6ac2cacb3e0a..2f14c6dc21844e2ba4a03b7861b5269bdd92aa6a 100644 (file)
--- a/src/ipcache.cc
+++ b/src/ipcache.cc
@@ -622,9 +622,9 @@ ipcacheHandleReply(void *data, rfc1035_rr * answers, int na, const char *error_m
  * of scheduling an async call. This reentrant behavior means that the
  * user job must be extra careful after calling ipcache_nbgethostbyname,
  * especially if the handler destroys the job. Moreover, the job has
- * no way of knowing whether the reentrant call happened. commConnectStart
- * protects the job by scheduling an async call, but some user code calls
- * ipcache_nbgethostbyname directly.
+ * no way of knowing whether the reentrant call happened.
+ * Comm::Connection setup usually protects the job by scheduling an async call,
+ * but some user code calls ipcache_nbgethostbyname directly.
  */
 void
 ipcache_nbgethostbyname(const char *name, IPH * handler, void *handlerData)

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 82ea74e03332b0386c65b96daa9af76aa043ad26..fd3402b152d645d119c74e4c890cfc9ed9f19d7a 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -33,8 +33,10 @@
 #include "squid.h"
 #include "ProtoPort.h"
 #include "acl/FilledChecklist.h"
-#include "event.h"
+#include "comm/Connection.h"
+#include "comm/ConnectStateData.h"
 #include "CacheManager.h"
+#include "event.h"
 #include "htcp.h"
 #include "HttpRequest.h"
 #include "ICP.h"
@@ -46,7 +48,6 @@
 #include "Store.h"
 #include "icmp/net_db.h"
 #include "ip/Address.h"
-#include "comm/ConnectStateData.h"
 
 /* count mcast group peers every 15 minutes */
 #define MCAST_COUNT_RATE 900
@@ -1391,7 +1392,7 @@ peerProbeConnectDone(Comm::ConnectionPointer conn, Comm::PathsPointer unused, co
         peerConnectFailedSilent(p);
     }
 
-    comm_close(conn);
+    conn->close();
     p->testing_now = false;
     return;
 }