From: Amos Jeffries Date: Wed, 9 Jun 2010 09:50:31 +0000 (+1200) Subject: Make comm_close a method of Comm::Connection X-Git-Tag: take08~55^2~124^2~134 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=55cbb02b85fe2f452c1c9f28c0d6f71ffd0375a4;p=thirdparty%2Fsquid.git Make comm_close a method of Comm::Connection --- diff --git a/src/comm.cc b/src/comm.cc index 18ca3ae634..d7bf01d583 100644 --- 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 f7f9a390f0..dd4e6ccaa5 100644 --- 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 951dc519c5..3d6e4ec485 100644 --- 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 9c325dc421..76da31315a 100644 --- 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 dfe7803c29..f495cd99f8 100644 --- 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 4fcda73b68..4070724700 100644 --- 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 edda02653a..9f3406a12c 100644 --- 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 acceptDialer; diff --git a/src/gopher.cc b/src/gopher.cc index 3e7b257ce0..35d695fc10 100644 --- 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 5fdf12159a..65c038bbd4 100644 --- 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 d69f41426d..2f14c6dc21 100644 --- 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 82ea74e033..fd3402b152 100644 --- 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; }