Cleanup: comm Close handlers

Make handlers take the CommCloseCbParams instead of series of expanded
variables.

Opening access to the other CommCommonCbParams fields with Connection/FD
data. Hiding the deprecated FD parameter from most handlers. Which seem
not to have actually needed it in most cases outside Comm.

diff --git a/src/CommCalls.cc b/src/CommCalls.cc
index d16d0af959ad68be5abff6c0070439fca1916424..77ca002d6c1a8a5de9ad24fa37e96930623963c9 100644 (file)
--- a/src/CommCalls.cc
+++ b/src/CommCalls.cc
@@ -188,7 +188,7 @@ CommIoCbPtrFun::print(std::ostream &os) const
 
 /* CommCloseCbPtrFun */
 
-CommCloseCbPtrFun::CommCloseCbPtrFun(PF *aHandler,
+CommCloseCbPtrFun::CommCloseCbPtrFun(CLCB *aHandler,
                                      const CommCloseCbParams &aParams):
         CommDialerParamsT<CommCloseCbParams>(aParams),
         handler(aHandler)
@@ -198,7 +198,7 @@ CommCloseCbPtrFun::CommCloseCbPtrFun(PF *aHandler,
 void
 CommCloseCbPtrFun::dial()
 {
-    handler(params.fd, params.data);
+    handler(params);
 }
 
 void

diff --git a/src/CommCalls.h b/src/CommCalls.h
index 065a8b3e32bf9c9afdebb7ca717a2932755385e8..ef4813a70002b3c0d96c3c64228acd6867ba4a24 100644 (file)
--- a/src/CommCalls.h
+++ b/src/CommCalls.h
@@ -20,6 +20,7 @@
  *     - connect (CNCB)
  *     - I/O (IOCB)
  *     - timeout (CTCB)
+ *     - close (CLCB)
  */
 
 class CommAcceptCbParams;
@@ -31,6 +32,9 @@ typedef void IOCB(const Comm::ConnectionPointer &conn, char *, size_t size, comm
 class CommTimeoutCbParams;
 typedef void CTCB(const CommTimeoutCbParams &params);
 
+class CommCloseCbParams;
+typedef void CLCB(const CommCloseCbParams &params);
+
 /*
  * TODO: When there are no function-pointer-based callbacks left, all
  * this complexity can be removed. Jobs that need comm services will just
@@ -233,20 +237,20 @@ public:
 };
 
 
-// close (PF) dialer
+// close (CLCB) dialer
 class CommCloseCbPtrFun: public CallDialer,
         public CommDialerParamsT<CommCloseCbParams>
 {
 public:
     typedef CommCloseCbParams Params;
 
-    CommCloseCbPtrFun(PF *aHandler, const Params &aParams);
+    CommCloseCbPtrFun(CLCB *aHandler, const Params &aParams);
     void dial();
 
     virtual void print(std::ostream &os) const;
 
 public:
-    PF *handler;
+    CLCB *handler;
 };
 
 class CommTimeoutCbPtrFun:public CallDialer,

diff --git a/src/CommRead.h b/src/CommRead.h
index da23379254d32128741d3a7d0a8646642c5f1f43..e206822db52fa75c95b56020ca975ba97843a59d 100644 (file)
--- a/src/CommRead.h
+++ b/src/CommRead.h
@@ -80,7 +80,7 @@ public:
     void kickReads(int const count);
 
 private:
-    static PF CloseHandler;
+    static CLCB CloseHandler;
     static DeferredRead popHead(CbDataListContainer<DeferredRead> &deferredReads);
     void kickARead(DeferredRead const &);
     void flushReads();

diff --git a/src/comm.cc b/src/comm.cc
index 3fc3a65774c75a70bd6b37bab560cb362799bd1d..7fb20fed7a9ea6d9326121d6da48985c2f7d0178 100644 (file)
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -962,6 +962,7 @@ commCallCloseHandlers(int fd)
         // If call is not canceled schedule it for execution else ignore it
         if (!call->canceled()) {
             debugs(5, 5, "commCallCloseHandlers: ch->handler=" << call);
+            // XXX: this should not be needed. Params can be set by the call creator
             typedef CommCloseCbParams Params;
             Params &params = GetCommParams<Params>(call);
             params.fd = fd;
@@ -998,10 +999,8 @@ void
 comm_lingering_close(int fd)
 {
 #if USE_SSL
-
     if (fd_table[fd].ssl)
-        ssl_shutdown_method(fd);
-
+        ssl_shutdown_method(fd_table[fd].ssl);
 #endif
 
     if (shutdown(fd, 1) < 0) {
@@ -1047,23 +1046,20 @@ old_comm_reset_close(int fd)
     comm_close(fd);
 }
 
+#if USE_SSL
 void
-comm_close_start(int fd, void *data)
+commStartSslClose(const CommCloseCbParams &params)
 {
-#if USE_SSL
-    fde *F = &fd_table[fd];
-    if (F->ssl)
-        ssl_shutdown_method(fd);
-
-#endif
-
+    assert(&fd_table[params.fd].ssl);
+    ssl_shutdown_method(fd_table[params.fd].ssl);
 }
+#endif
 
 void
-comm_close_complete(int fd, void *data)
+comm_close_complete(const CommCloseCbParams &params)
 {
 #if USE_SSL
-    fde *F = &fd_table[fd];
+    fde *F = &fd_table[params.fd];
 
     if (F->ssl) {
         SSL_free(F->ssl);
@@ -1075,13 +1071,12 @@ comm_close_complete(int fd, void *data)
         F->dynamicSslContext = NULL;
     }
 #endif
-    fd_close(fd);              /* update fdstat */
-
-    close(fd);
+    fd_close(params.fd);               /* update fdstat */
+    close(params.fd);
 
     statCounter.syscalls.sock.closes++;
 
-    /* When an fd closes, give accept() a chance, if need be */
+    /* When one connection closes, give accept() a chance, if need be */
     Comm::AcceptLimiter::Instance().kick();
 }
 
@@ -1122,12 +1117,16 @@ _comm_close(int fd, char const *file, int line)
 
     F->flags.close_request = 1;
 
-    AsyncCall::Pointer startCall=commCbCall(5,4, "comm_close_start",
-                                            CommCloseCbPtrFun(comm_close_start, NULL));
-    typedef CommCloseCbParams Params;
-    Params &startParams = GetCommParams<Params>(startCall);
-    startParams.fd = fd;
-    ScheduleCallHere(startCall);
+#if USE_SSL
+    if (F->ssl) {
+        // XXX: make this a generic async call passing one FD parameter. No need to use CommCloseCbParams
+        AsyncCall::Pointer startCall=commCbCall(5,4, "commStartSslClose",
+                                                CommCloseCbPtrFun(commStartSslClose, NULL));
+        CommCloseCbParams &startParams = GetCommParams<CommCloseCbParams>(startCall);
+        startParams.fd = fd;
+        ScheduleCallHere(startCall);
+    }
+#endif
 
     // a half-closed fd may lack a reader, so we stop monitoring explicitly
     if (commHasHalfClosedMonitor(fd))
@@ -1164,7 +1163,7 @@ _comm_close(int fd, char const *file, int line)
 
     AsyncCall::Pointer completeCall=commCbCall(5,4, "comm_close_complete",
                                     CommCloseCbPtrFun(comm_close_complete, NULL));
-    Params &completeParams = GetCommParams<Params>(completeCall);
+    CommCloseCbParams &completeParams = GetCommParams<CommCloseCbParams>(completeCall);
     completeParams.fd = fd;
     // must use async call to wait for all callbacks
     // scheduled before comm_close() to finish
@@ -1215,7 +1214,7 @@ comm_udp_sendto(int fd,
 }
 
 void
-comm_add_close_handler(int fd, PF * handler, void *data)
+comm_add_close_handler(int fd, CLCB * handler, void *data)
 {
     debugs(5, 5, "comm_add_close_handler: FD " << fd << ", handler=" <<
            handler << ", data=" << data);
@@ -1242,7 +1241,7 @@ comm_add_close_handler(int fd, AsyncCall::Pointer &call)
 
 // remove function-based close handler
 void
-comm_remove_close_handler(int fd, PF * handler, void *data)
+comm_remove_close_handler(int fd, CLCB * handler, void *data)
 {
     assert (isOpen(fd));
     /* Find handler in list */
@@ -2022,6 +2021,7 @@ DeferredReadManager::delayRead(DeferredRead const &aRead)
     // We have to use a global function as a closer and point to temp
     // instead of "this" because DeferredReadManager is not a job and
     // is not even cbdata protected
+    // XXX: and yet we use cbdata protection functions on it??
     AsyncCall::Pointer closer = commCbCall(5,4,
                                            "DeferredReadManager::CloseHandler",
                                            CommCloseCbPtrFun(&CloseHandler, temp));
@@ -2030,12 +2030,12 @@ DeferredReadManager::delayRead(DeferredRead const &aRead)
 }
 
 void
-DeferredReadManager::CloseHandler(int fd, void *thecbdata)
+DeferredReadManager::CloseHandler(const CommCloseCbParams &params)
 {
-    if (!cbdataReferenceValid (thecbdata))
+    if (!cbdataReferenceValid(params.data))
         return;
 
-    CbDataList<DeferredRead> *temp = (CbDataList<DeferredRead> *)thecbdata;
+    CbDataList<DeferredRead> *temp = (CbDataList<DeferredRead> *)params.data;
 
     temp->element.closer = NULL;
     temp->element.markCancelled();

diff --git a/src/comm.h b/src/comm.h
index 98c58e403b3543dc8e0dd8c6eb2e3b1c18f60217..5e79dd486aabe34e385d065f1f05b8f310655150 100644 (file)
--- a/src/comm.h
+++ b/src/comm.h
@@ -72,9 +72,9 @@ SQUIDCEXTERN void checkTimeouts(void);
 
 
 //typedef void IOACB(int fd, int nfd, Comm::ConnectionPointer details, comm_err_t flag, int xerrno, void *data);
-extern void comm_add_close_handler(int fd, PF *, void *);
+extern void comm_add_close_handler(int fd, CLCB *, void *);
 extern void comm_add_close_handler(int fd, AsyncCall::Pointer &);
-extern void comm_remove_close_handler(int fd, PF *, void *);
+extern void comm_remove_close_handler(int fd, CLCB *, void *);
 extern void comm_remove_close_handler(int fd, AsyncCall::Pointer &);
 
 

diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index 4c3f7ab314c18a1a87bae5f1c19ca8d83d39335f..91de65a44c95e25f3b2d35172d06c3d6dc9a361b 100644 (file)
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -247,7 +247,7 @@ static PF idnsRead;
 static EVH idnsCheckQueue;
 static void idnsTickleQueue(void);
 static void idnsRcodeCount(int, int);
-static void idnsVCClosed(int fd, void *data);
+static CLCB idnsVCClosed;
 static unsigned short idnsQueryID(void);
 
 static void
@@ -815,9 +815,9 @@ idnsInitVCConnected(const Comm::ConnectionPointer &conn, comm_err_t status, int
 }
 
 static void
-idnsVCClosed(int fd, void *data)
+idnsVCClosed(const CommCloseCbParams &params)
 {
-    nsvc * vc = (nsvc *)data;
+    nsvc * vc = (nsvc *)params.data;
     delete vc->queue;
     delete vc->msg;
     vc->conn = NULL;

diff --git a/src/forward.cc b/src/forward.cc
index 7bd660699f5fb058e251ac4a837dfce383838ed9..56f3c65471cdf70e7533e00e588469d9566cfb63 100644 (file)
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -62,7 +62,7 @@
 #endif
 
 static PSC fwdPeerSelectionCompleteWrapper;
-static PF fwdServerClosedWrapper;
+static CLCB fwdServerClosedWrapper;
 #if USE_SSL
 static PF fwdNegotiateSSLWrapper;
 #endif
@@ -417,10 +417,10 @@ fwdPeerSelectionCompleteWrapper(Comm::ConnectionList * unused, void *data)
 }
 
 static void
-fwdServerClosedWrapper(int fd, void *data)
+fwdServerClosedWrapper(const CommCloseCbParams &params)
 {
-    FwdState *fwd = (FwdState *) data;
-    fwd->serverClosed(fd);
+    FwdState *fwd = (FwdState *)params.data;
+    fwd->serverClosed(params.fd);
 }
 
 #if USE_SSL

diff --git a/src/gopher.cc b/src/gopher.cc
index 52ef9adfdf308b5e65bd294b43d9d0a86aa357fd..0eb0d1e6465357fbf0f57c6744712674e80ee1d8 100644 (file)
--- a/src/gopher.cc
+++ b/src/gopher.cc
@@ -141,7 +141,7 @@ typedef struct gopher_ds {
     char replybuf[BUFSIZ];
 } GopherStateData;
 
-static PF gopherStateFree;
+static CLCB gopherStateFree;
 static void gopherMimeCreate(GopherStateData *);
 static void gopher_request_parse(const HttpRequest * req,
                                  char *type_id,
@@ -161,9 +161,9 @@ static char def_gopher_text[] = "text/plain";
 
 /// \ingroup ServerProtocolGopherInternal
 static void
-gopherStateFree(int, void *data)
+gopherStateFree(const CommCloseCbParams &params)
 {
-    GopherStateData *gopherState = (GopherStateData *)data;
+    GopherStateData *gopherState = (GopherStateData *)params.data;
 
     if (gopherState == NULL)
         return;

diff --git a/src/helper.cc b/src/helper.cc
index 6a1ce2aca4036e186507b478535e7ddb11c00814..724fd8b4d0a4808a11f9afa481bea6267466c45f 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -53,8 +53,8 @@
 
 static IOCB helperHandleRead;
 static IOCB helperStatefulHandleRead;
-static PF helperServerFree;
-static PF helperStatefulServerFree;
+static CLCB helperServerFree;
+static CLCB helperStatefulServerFree;
 static void Enqueue(helper * hlp, helper_request *);
 static helper_request *Dequeue(helper * hlp);
 static helper_stateful_request *StatefulDequeue(statefulhelper * hlp);
@@ -669,9 +669,9 @@ helper::~helper()
 /* ====================================================================== */
 
 static void
-helperServerFree(int fd, void *data)
+helperServerFree(const CommCloseCbParams &params)
 {
-    helper_server *srv = (helper_server *)data;
+    helper_server *srv = (helper_server *)params.data;
     helper *hlp = srv->parent;
     helper_request *r;
     int i, concurrency = hlp->childs.concurrency;
@@ -704,7 +704,7 @@ helperServerFree(int fd, void *data)
     if (!srv->flags.shutdown) {
         assert(hlp->childs.n_active > 0);
         hlp->childs.n_active--;
-        debugs(84, DBG_CRITICAL, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << fd << ") exited");
+        debugs(84, DBG_CRITICAL, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << params.fd << ") exited");
 
         if (hlp->childs.needNew() > 0) {
             debugs(80, 1, "Too few " << hlp->id_name << " processes are running (need " << hlp->childs.needNew() << "/" << hlp->childs.n_max << ")");
@@ -736,9 +736,9 @@ helperServerFree(int fd, void *data)
 }
 
 static void
-helperStatefulServerFree(int fd, void *data)
+helperStatefulServerFree(const CommCloseCbParams &params)
 {
-    helper_stateful_server *srv = (helper_stateful_server *)data;
+    helper_stateful_server *srv = (helper_stateful_server *)params.data;
     statefulhelper *hlp = srv->parent;
     helper_stateful_request *r;
 
@@ -766,7 +766,7 @@ helperStatefulServerFree(int fd, void *data)
     if (!srv->flags.shutdown) {
         assert( hlp->childs.n_active > 0);
         hlp->childs.n_active--;
-        debugs(84, 0, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << fd << ") exited");
+        debugs(84, 0, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << params.fd << ") exited");
 
         if (hlp->childs.needNew() > 0) {
             debugs(80, 1, "Too few " << hlp->id_name << " processes are running (need " << hlp->childs.needNew() << "/" << hlp->childs.n_max << ")");

diff --git a/src/ident/Ident.cc b/src/ident/Ident.cc
index ace6e3c0dd1308bf6358a8e5b93681791e06bb23..ec3ee17bd17e4136fc3c23cc64ddea20dbd0c65b 100644 (file)
--- a/src/ident/Ident.cc
+++ b/src/ident/Ident.cc
@@ -68,7 +68,7 @@ typedef struct _IdentStateData {
 
 // TODO: make these all a series of Async job calls. They are self-contained callbacks now.
 static IOCB ReadReply;
-static PF Close;
+static CLCB Close;
 static CTCB Timeout;
 static CNCB ConnectDone;
 static hash_table *ident_hash = NULL;
@@ -101,9 +101,9 @@ Ident::identCallback(IdentStateData * state, char *result)
 }
 
 void
-Ident::Close(int fdnotused, void *data)
+Ident::Close(const CommCloseCbParams &params)
 {
-    IdentStateData *state = (IdentStateData *)data;
+    IdentStateData *state = (IdentStateData *)params.data;
     identCallback(state, NULL);
     state->conn->close();
     hash_remove_link(ident_hash, (hash_link *) state);

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index a5b5ab1e52bdb1cfeef86ef277abcec4b1925cc0..dc961c010371121a49f524caf8f4c5672cdfd064 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1019,10 +1019,8 @@ ssl_write_method(int fd, const char *buf, int len)
 }
 
 void
-ssl_shutdown_method(int fd)
+ssl_shutdown_method(SSL *ssl)
 {
-    SSL *ssl = fd_table[fd].ssl;
-
     SSL_shutdown(ssl);
 }
 

diff --git a/src/ssl/support.h b/src/ssl/support.h
index beeb87928ac6e08e2e3160fd4d8b7ac49784b713..c84b4cf460728fb48c117011ec7ce1f8a42247a2 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -81,7 +81,7 @@ int ssl_read_method(int, char *, int);
 int ssl_write_method(int, const char *, int);
 
 /// \ingroup ServerProtocolSSLAPI
-void ssl_shutdown_method(int);
+void ssl_shutdown_method(SSL *ssl);
 
 
 /// \ingroup ServerProtocolSSLAPI

diff --git a/src/tunnel.cc b/src/tunnel.cc
index 6c46e8e3104171ca2c1f099509974cbcc10474a7..3344f799260ffa1df4f9f742df1dc87c7440483d 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -124,8 +124,8 @@ static const char *const conn_established = "HTTP/1.1 200 Connection established
 
 static CNCB tunnelConnectDone;
 static ERCB tunnelErrorComplete;
-static PF tunnelServerClosed;
-static PF tunnelClientClosed;
+static CLCB tunnelServerClosed;
+static CLCB tunnelClientClosed;
 static CTCB tunnelTimeout;
 static PSC tunnelPeerSelectComplete;
 static void tunnelStateFree(TunnelStateData * tunnelState);
@@ -133,10 +133,10 @@ static void tunnelConnected(const Comm::ConnectionPointer &server, void *);
 static void tunnelRelayConnectRequest(const Comm::ConnectionPointer &server, void *);
 
 static void
-tunnelServerClosed(int fd, void *data)
+tunnelServerClosed(const CommCloseCbParams &params)
 {
-    TunnelStateData *tunnelState = (TunnelStateData *)data;
-    debugs(26, 3, HERE << "FD " << fd);
+    TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+    debugs(26, 3, HERE << tunnelState->server.conn);
     tunnelState->server.conn = NULL;
 
     if (tunnelState->noConnections()) {
@@ -151,10 +151,10 @@ tunnelServerClosed(int fd, void *data)
 }
 
 static void
-tunnelClientClosed(int fd, void *data)
+tunnelClientClosed(const CommCloseCbParams &params)
 {
-    TunnelStateData *tunnelState = (TunnelStateData *)data;
-    debugs(26, 3, HERE << "FD " << fd);
+    TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+    debugs(26, 3, HERE << tunnelState->client.conn);
     tunnelState->client.conn = NULL;
 
     if (tunnelState->noConnections()) {

diff --git a/src/whois.cc b/src/whois.cc
index 82dbc0f646e33a322f980e49ab4b74fc381c8484..ad30fbfba0ad434d5836ebcabbc02b5973d93af1 100644 (file)
--- a/src/whois.cc
+++ b/src/whois.cc
@@ -59,7 +59,7 @@ public:
     bool dataWritten;
 };
 
-static PF whoisClose;
+static CLCB whoisClose;
 static CTCB whoisTimeout;
 static IOCB whoisReadReply;
 
@@ -197,10 +197,10 @@ WhoisState::readReply(const Comm::ConnectionPointer &conn, char *aBuffer, size_t
 }
 
 static void
-whoisClose(int fd, void *data)
+whoisClose(const CommCloseCbParams &params)
 {
-    WhoisState *p = (WhoisState *)data;
-    debugs(75, 3, "whoisClose: FD " << fd);
+    WhoisState *p = (WhoisState *)params.data;
+    debugs(75, 3, "whoisClose: FD " << params.fd);
     p->entry->unlock();
     cbdataFree(p);
 }