]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Protect SMP kids from unsolicited IPC answers (#771)
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Fri, 12 Feb 2021 13:33:16 +0000 (13:33 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Fri, 12 Feb 2021 21:00:12 +0000 (21:00 +0000)
When an SMP kid restarts, it recreates its IPC socket to flush any old
messages, but it might still receive IPC messages intended for its
previous OS process because the sender may write the message _after_
kid's IPC socket is flushed. Squid IPC communication is connectionless,
so the sender cannot easily detect the reopening of the recipient socket
to prevent this race condition. Some notifications are desirable across
kid restarts, so properly switching to connection-oriented IPC
communication would only complicate things further.

This change protects kids from, for the lack of a better term, an
"unsolicited" answer: An answer to a question the recipient did not ask.
When allowed to reach regular message-handling code, unsolicited answers
result in misleading diagnostics, may trigger assertions, and might even
result in a bad recipient state. For example, a kid might think it has
been successfully registered with Coordinator while its registration
attempt was actually dropped due to a Coordinator restart.

Our protection targets one specific use case: Responses that were born
(or became) "unsolicited" due to a recipient restart. Other problematic
cases may not require any protection, may require a very different
protection mechanism (e.g. cryptography), may deal with requests rather
than responses, or even cannot be reliably detected. For example:

* messages sent by a malicious attacker
* requests sent by a misconfigured Squid instance
* requests sent by a previous Squid instance
* messages sent by a no-longer-running kid process
* messages sent by buggy Squid code

----

Also marked a few out-of-scope problems/improvements, including a bug:

Improved handling of Coordinator and Strand exceptions exposed and
partially addressed an old problem: When configured to listen on an
inaccessible port, Squid used to kill the Coordinator job, resulting in
subsequent kid registration timeouts. Squid now correctly keeps the
Coordinator job running, logging a detailed report:

    WARNING: Ignoring problematic IPC message
        message type: 5
        problem: check failed: fd >= 0
        exception location: TypedMsgHdr.cc(198) putFd

Still, the affected kids do not report the port opening problem and
continue running, listening on the problem-free ports (if any).
Depending on the exception timing, similar behavior was possible before
these changes. The correct action here is to send the port opening error
to the kid process without throwing the putFd() exception, but we should
decide how Squid should handle such inaccessible configured ports first.

45 files changed:
src/DiskIO/IpcIo/IpcIoFile.cc
src/base/Makefile.am
src/base/TypeTraits.h [new file with mode: 0644]
src/ip/Address.h
src/ipc/Coordinator.cc
src/ipc/Forwarder.cc
src/ipc/Forwarder.h
src/ipc/Inquirer.cc
src/ipc/Inquirer.h
src/ipc/Makefile.am
src/ipc/Port.cc
src/ipc/Port.h
src/ipc/QuestionerId.cc [new file with mode: 0644]
src/ipc/QuestionerId.h [new file with mode: 0644]
src/ipc/Request.h
src/ipc/RequestId.cc [new file with mode: 0644]
src/ipc/RequestId.h [new file with mode: 0644]
src/ipc/Response.h
src/ipc/SharedListen.cc
src/ipc/SharedListen.h
src/ipc/Strand.cc
src/ipc/StrandCoord.cc
src/ipc/StrandCoord.h
src/ipc/StrandSearch.cc
src/ipc/StrandSearch.h
src/ipc/TypedMsgHdr.h
src/ipc/forward.h
src/mgr/Action.cc
src/mgr/Action.h
src/mgr/Filler.cc
src/mgr/Filler.h
src/mgr/Forwarder.cc
src/mgr/FunAction.cc
src/mgr/InfoAction.cc
src/mgr/Request.cc
src/mgr/Request.h
src/mgr/Response.cc
src/mgr/Response.h
src/snmp/Forwarder.cc
src/snmp/Forwarder.h
src/snmp/Request.cc
src/snmp/Request.h
src/snmp/Response.cc
src/snmp/Response.h
src/tests/stub_libmgr.cc

index 90c8d9cb35fa323b32a63ee03b5219c57378eeeb..67b94985374f9be15a5d02dc11f803d98f4a2e56 100644 (file)
@@ -150,10 +150,7 @@ IpcIoFile::open(int flags, mode_t mode, RefCount<IORequestor> callback)
         return;
     }
 
-    Ipc::StrandSearchRequest request;
-    request.requestorId = KidIdentifier;
-    request.tag = dbName;
-
+    const Ipc::StrandSearchRequest request(dbName);
     Ipc::TypedMsgHdr msg;
     request.pack(msg);
     Ipc::SendMessage(Ipc::Port::CoordinatorAddr(), msg);
index 4792522f8dfe9c98f8470d79df70124f5a2528df..1366df5b7589187b776a0731e8e0cda9e5e3cc97 100644 (file)
@@ -49,5 +49,6 @@ libbase_la_SOURCES = \
        Subscription.h \
        TextException.cc \
        TextException.h \
+       TypeTraits.h \
        YesNoNone.h \
        forward.h
diff --git a/src/base/TypeTraits.h b/src/base/TypeTraits.h
new file mode 100644 (file)
index 0000000..719fcee
--- /dev/null
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_SRC_BASE_TYPETRAITS_H
+#define SQUID_SRC_BASE_TYPETRAITS_H
+
+namespace TypeTraits_ { // a hack to prevent "unintended ADL"
+
+// TODO: Extract reusable paradigms into other mixins (e.g., NonCopyable).
+/// convenience base for any class with pure virtual method(s)
+class Interface
+{
+public:
+    // ensures proper destruction via pointers to base interface classes
+    virtual ~Interface() = default;
+
+    // prohibits copy/move assignment to prevent accidental object slicing
+    Interface &operator=(const Interface &) = delete;
+    Interface &operator=(Interface &&) = delete;
+
+protected: // prevents accidental creation of Interface instances
+
+    // allows default-construction in kids
+    constexpr Interface() = default;
+
+    // allows copy/move construction for kids convenience
+    Interface(const Interface &) = default;
+    Interface(Interface &&) = default;
+};
+
+} // namespace TypeTraits_
+
+using Interface = TypeTraits_::Interface;
+
+#endif /* SQUID_SRC_BASE_TYPETRAITS_H */
+
index 7555e28ff5d3f49c1bd4eb9b4437a44ae8ff1391..d40f7f855d0a36df8fd75fb9b42a178a103eb658 100644 (file)
@@ -41,7 +41,7 @@ class Address
 {
 
 public:
-    /** @name Constructors and Destructor */
+    /** @name Constructors */
     /*@{*/
     Address() { setEmpty(); }
     Address(const struct in_addr &);
@@ -51,7 +51,6 @@ public:
     Address(const struct hostent &);
     Address(const struct addrinfo &);
     Address(const char*);
-    ~Address() {}
     /*@}*/
 
     /** @name Assignment Operators */
index 012e6fb8e26d250bbc604ef18f4ef3e830b02ab4..6202cd175be4bb581e3960267322e65bec5b9dff 100644 (file)
@@ -107,7 +107,7 @@ void Ipc::Coordinator::receive(const TypedMsgHdr& message)
     case mtCacheMgrResponse: {
         debugs(54, 6, HERE << "Cache manager response");
         const Mgr::Response resp(message);
-        handleCacheMgrResponse(resp);
+        handleCacheMgrResponse(Mine(resp));
     }
     break;
 
@@ -122,13 +122,13 @@ void Ipc::Coordinator::receive(const TypedMsgHdr& message)
     case mtSnmpResponse: {
         debugs(54, 6, HERE << "SNMP response");
         const Snmp::Response resp(message);
-        handleSnmpResponse(resp);
+        handleSnmpResponse(Mine(resp));
     }
     break;
 #endif
 
     default:
-        debugs(54, DBG_IMPORTANT, "WARNING: Ignoring IPC message with an unknown type: " << message.rawType());
+        Port::receive(message);
         break;
     }
 }
@@ -222,7 +222,7 @@ Ipc::Coordinator::notifySearcher(const Ipc::StrandSearchRequest &request,
 {
     debugs(54, 3, HERE << "tell kid" << request.requestorId << " that " <<
            request.tag << " is kid" << strand.kidId);
-    const StrandMessage response(strand);
+    const StrandMessage response(strand, request.qid);
     TypedMsgHdr message;
     response.pack(mtStrandReady, message);
     SendMessage(MakeAddr(strandAddrLabel, request.requestorId), message);
index 70ec73a7b8fee37cf39acb0be209658d9ad59610..46df988763a0a90f4e88cfed075101e65a7b1115 100644 (file)
 #include "HttpRequest.h"
 #include "ipc/Forwarder.h"
 #include "ipc/Port.h"
+#include "ipc/RequestId.h"
 #include "ipc/TypedMsgHdr.h"
 
 CBDATA_NAMESPACED_CLASS_INIT(Ipc, Forwarder);
 
 Ipc::Forwarder::RequestsMap Ipc::Forwarder::TheRequestsMap;
-unsigned int Ipc::Forwarder::LastRequestId = 0;
+Ipc::RequestId::Index Ipc::Forwarder::LastRequestId = 0;
 
 Ipc::Forwarder::Forwarder(Request::Pointer aRequest, double aTimeout):
     AsyncJob("Ipc::Forwarder"),
@@ -149,7 +150,7 @@ Ipc::Forwarder::callException(const std::exception& e)
 
 /// returns and forgets the right Forwarder callback for the request
 AsyncCall::Pointer
-Ipc::Forwarder::DequeueRequest(unsigned int requestId)
+Ipc::Forwarder::DequeueRequest(const RequestId::Index requestId)
 {
     debugs(54, 3, HERE);
     Must(requestId != 0);
@@ -172,7 +173,7 @@ Ipc::Forwarder::removeTimeoutEvent()
 }
 
 void
-Ipc::Forwarder::HandleRemoteAck(unsigned int requestId)
+Ipc::Forwarder::HandleRemoteAck(const RequestId requestId)
 {
     debugs(54, 3, HERE);
     Must(requestId != 0);
index 69c957b773eee1f6b821dfe7c15bf6760864f845..7179619a178e6089aa45f9a6a090f0510f8f82d3 100644 (file)
@@ -35,7 +35,7 @@ public:
     virtual ~Forwarder();
 
     /// finds and calls the right Forwarder upon Coordinator's response
-    static void HandleRemoteAck(unsigned int requestId);
+    static void HandleRemoteAck(RequestId);
 
     /* has-to-be-public AsyncJob API */
     virtual void callException(const std::exception& e);
@@ -59,17 +59,17 @@ private:
 
     void handleRemoteAck();
 
-    static AsyncCall::Pointer DequeueRequest(unsigned int requestId);
+    static AsyncCall::Pointer DequeueRequest(RequestId::Index);
 
 protected:
     Request::Pointer request;
     const double timeout; ///< response wait timeout in seconds
 
     /// maps request->id to Forwarder::handleRemoteAck callback
-    typedef std::map<unsigned int, AsyncCall::Pointer> RequestsMap;
+    typedef std::map<RequestId::Index, AsyncCall::Pointer> RequestsMap;
     static RequestsMap TheRequestsMap; ///< pending Coordinator requests
 
-    static unsigned int LastRequestId; ///< last requestId used
+    static RequestId::Index LastRequestId; ///< last requestId used
 };
 
 } // namespace Ipc
index 6ce807ce3f07a73e062895856c275a913eaed8b8..481710c47a066861b9b1dbdeb3f47cfc84338a97 100644 (file)
@@ -21,7 +21,7 @@
 CBDATA_NAMESPACED_CLASS_INIT(Ipc, Inquirer);
 
 Ipc::Inquirer::RequestsMap Ipc::Inquirer::TheRequestsMap;
-unsigned int Ipc::Inquirer::LastRequestId = 0;
+Ipc::RequestId::Index Ipc::Inquirer::LastRequestId = 0;
 
 /// compare Ipc::StrandCoord using kidId, for std::sort() below
 static bool
@@ -139,7 +139,7 @@ Ipc::Inquirer::callException(const std::exception& e)
 
 /// returns and forgets the right Inquirer callback for strand request
 AsyncCall::Pointer
-Ipc::Inquirer::DequeueRequest(unsigned int requestId)
+Ipc::Inquirer::DequeueRequest(const RequestId::Index requestId)
 {
     debugs(54, 3, HERE << " requestId " << requestId);
     Must(requestId != 0);
@@ -206,7 +206,7 @@ Ipc::Inquirer::status() const
 {
     static MemBuf buf;
     buf.reset();
-    buf.appendf(" [request->requestId %u]", request->requestId);
+    buf.appendf(" [requestId %u]", request->requestId.index());
     buf.terminate();
     return buf.content();
 }
index 527252aa601ce82e9f76820b7f93f4cc7353eed6..d97ea884a56830ecc760442c7914110e92c2a235 100644 (file)
@@ -64,7 +64,7 @@ private:
 
     void handleRemoteAck(Response::Pointer response);
 
-    static AsyncCall::Pointer DequeueRequest(unsigned int requestId);
+    static AsyncCall::Pointer DequeueRequest(RequestId::Index);
 
     static void RequestTimedOut(void* param);
     void requestTimedOut();
@@ -79,10 +79,10 @@ protected:
     const double timeout; ///< number of seconds to wait for strand response
 
     /// maps request->id to Inquirer::handleRemoteAck callback
-    typedef std::map<unsigned int, AsyncCall::Pointer> RequestsMap;
+    typedef std::map<RequestId::Index, AsyncCall::Pointer> RequestsMap;
     static RequestsMap TheRequestsMap; ///< pending strand requests
 
-    static unsigned int LastRequestId; ///< last requestId used
+    static RequestId::Index LastRequestId; ///< last requestId used
 };
 
 } // namespace Ipc
index e84da0db72492822721cf3065222c44e72663554..dd9ec392ff673ec43cd8867f024d664e2484faf2 100644 (file)
@@ -28,11 +28,15 @@ libipc_la_SOURCES = \
        Messages.h \
        Port.cc \
        Port.h \
+       QuestionerId.cc \
+       QuestionerId.h \
        Queue.cc \
        Queue.h \
        ReadWriteLock.cc \
        ReadWriteLock.h \
        Request.h \
+       RequestId.cc \
+       RequestId.h \
        Response.h \
        SharedListen.cc \
        SharedListen.h \
index f3d1a3853f854ce8f6545107fa508edd2b228d49..3e9f06b4366a1e779895cf134182ec00fcd5f571 100644 (file)
@@ -14,6 +14,7 @@
 #include "comm/Read.h"
 #include "CommCalls.h"
 #include "ipc/Port.h"
+#include "sbuf/Stream.h"
 #include "tools.h"
 #include "util.h"
 
@@ -73,6 +74,25 @@ Ipc::Port::CoordinatorAddr()
     return coordinatorAddr;
 }
 
+void
+Ipc::Port::receive(const TypedMsgHdr &message)
+{
+    throw TextException(ToSBuf("bad IPC message type: ", message.rawType()), Here());
+}
+
+/// receive() but ignore any errors
+void
+Ipc::Port::receiveOrIgnore(const TypedMsgHdr &message)
+{
+    try {
+        receive(message);
+    } catch (...) {
+        debugs(54, DBG_IMPORTANT, "WARNING: Ignoring IPC message" <<
+               Debug::Extra << "message type: " << message.rawType() <<
+               Debug::Extra << "problem: " << CurrentException);
+    }
+}
+
 void Ipc::Port::noteRead(const CommIoCbParams& params)
 {
     debugs(54, 6, HERE << params.conn << " flag " << params.flag <<
@@ -80,7 +100,7 @@ void Ipc::Port::noteRead(const CommIoCbParams& params)
     if (params.flag == Comm::OK) {
         assert(params.buf == buf.raw());
         debugs(54, 6, "message type: " << buf.rawType());
-        receive(buf);
+        receiveOrIgnore(buf);
     }
     // TODO: if there was a fatal error on our socket, close the socket before
     // trying to listen again and print a level-1 error message.
index df9e9b9064da53f6d16e145d7e84571a7184ba50..9f8e6ae9d50ff55b5a649b24514db24178c4752e 100644 (file)
@@ -36,10 +36,12 @@ protected:
     void doListen();
 
     /// handle IPC message just read
-    virtual void receive(const TypedMsgHdr& message) = 0;
+    /// kids must call parent method when they do not recognize the message type
+    virtual void receive(const TypedMsgHdr &) = 0;
 
 private:
     void noteRead(const CommIoCbParams &params); // Comm callback API
+    void receiveOrIgnore(const TypedMsgHdr& );
 
 private:
     TypedMsgHdr buf; ///< msghdr struct filled by Comm
diff --git a/src/ipc/QuestionerId.cc b/src/ipc/QuestionerId.cc
new file mode 100644 (file)
index 0000000..c530bb9
--- /dev/null
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "base/TextException.h"
+#include "ipc/QuestionerId.h"
+#include "ipc/TypedMsgHdr.h"
+#include "sbuf/Stream.h"
+
+#include <iostream>
+
+Ipc::QuestionerId
+Ipc::MyQuestionerId()
+{
+    static const QuestionerId qid(getpid());
+    return qid;
+}
+
+void
+Ipc::QuestionerId::pack(TypedMsgHdr &hdrMsg) const
+{
+    hdrMsg.putPod(pid);
+}
+
+void
+Ipc::QuestionerId::unpack(const TypedMsgHdr &hdrMsg)
+{
+    hdrMsg.getPod(pid);
+}
+
+void
+Ipc::QuestionerId::rejectAnswerIfStale() const
+{
+    const auto myPid = MyQuestionerId().pid;
+    if (myPid != pid) {
+        throw TextException(ToSBuf("received answer to an IPC question asked by process ", pid,
+                                   Debug::Extra, "my process PID: ", myPid), Here());
+    }
+}
+
+void
+Ipc::QuestionerId::print(std::ostream &os) const
+{
+    os << pid;
+}
+
diff --git a/src/ipc/QuestionerId.h b/src/ipc/QuestionerId.h
new file mode 100644 (file)
index 0000000..ee0efff
--- /dev/null
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_SRC_IPC_QUESTIONERID_H
+#define SQUID_SRC_IPC_QUESTIONERID_H
+
+#include "ipc/forward.h"
+
+#include <iosfwd>
+
+namespace Ipc
+{
+
+/// Identifies a kid process sending IPC messages that require an answer.
+/// Must be unique across all kids with pending questions.
+class QuestionerId
+{
+public:
+    /// to-be-determined ID
+    QuestionerId() = default;
+
+    /// for sending the ID of the asking process
+    void pack(TypedMsgHdr &) const;
+
+    /// for receiving the ID of the asking process
+    void unpack(const TypedMsgHdr &);
+
+    /// does nothing but throws if the questioner was not the current process
+    void rejectAnswerIfStale() const;
+
+    /// reports the stored opaque ID value (for debugging)
+    void print(std::ostream &) const;
+
+private:
+    /// for MyQuestionerId() convenience
+    explicit QuestionerId(const pid_t aPid): pid(aPid) {}
+    friend QuestionerId MyQuestionerId();
+
+    /// OS process ID of the asking kid. If the kid restarts, it is assumed
+    /// not to wrap back to the old value until the answer is received.
+    pid_t pid = -1;
+};
+
+/// the questioner ID of the current/calling process
+QuestionerId MyQuestionerId();
+
+/// Convenience wrapper for rejecting (freshly parsed) stale answers.
+/// All answers are assumed to have a "QuestionerId intendedRecepient()" member.
+template <class Answer>
+const Answer &
+Mine(const Answer &answer)
+{
+    answer.intendedRecepient().rejectAnswerIfStale();
+    return answer;
+}
+
+inline std::ostream &
+operator <<(std::ostream &os, const QuestionerId &qid)
+{
+    qid.print(os);
+    return os;
+}
+
+} // namespace Ipc;
+
+#endif /* SQUID_SRC_IPC_QUESTIONERID_H */
+
index 31006364f966e5c1c4921bf7c89606ae4b2881e7..089c190c5df5ff979505a6ffa64f6809c406bb1c 100644 (file)
 #define SQUID_IPC_REQUEST_H
 
 #include "base/RefCount.h"
-#include "ipc/forward.h"
+#include "base/TypeTraits.h"
+#include "ipc/RequestId.h"
 
 namespace Ipc
 {
 
+// TODO: Request and Response ought to have their own un/pack() methods instead
+// of duplicating their functionality in derived classes. To avoid dependency
+// loops between libipc and libmgr/libsnmp, fixing that requires extracting
+// src/ipc/Coordinator and its friends into a new src/coordinator/ library.
+
 /// IPC request
-class Request: public RefCountable
+class Request: public RefCountable, public Interface
 {
 public:
     typedef RefCount<Request> Pointer;
 
 public:
-    Request(int aRequestorId, unsigned int aRequestId):
-        requestorId(aRequestorId), requestId(aRequestId) {}
-
     virtual void pack(TypedMsgHdr& msg) const = 0; ///< prepare for sendmsg()
     virtual Pointer clone() const = 0; ///< returns a copy of this
 
-private:
-    Request(const Request&); // not implemented
-    Request& operator= (const Request&); // not implemented
-
 public:
-    int requestorId; ///< kidId of the requestor; used for response destination
-    unsigned int requestId; ///< unique for sender; matches request w/ response
+    int requestorId = 0; ///< kidId of the requestor; used for response destination
+    RequestId requestId; ///< matches the request[or] with the response
+
+protected:
+    /// sender's constructor
+    Request(const int aRequestorId, const RequestId aRequestId):
+        requestorId(aRequestorId),
+        requestId(aRequestId)
+    {
+    }
+
+    /// recipient's constructor
+    Request() = default;
 };
 
 } // namespace Ipc
diff --git a/src/ipc/RequestId.cc b/src/ipc/RequestId.cc
new file mode 100644 (file)
index 0000000..c27cb4f
--- /dev/null
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "base/TextException.h"
+#include "Debug.h"
+#include "ipc/RequestId.h"
+
+#include <iostream>
+
+Ipc::RequestId::RequestId(const Index anIndex):
+    qid_(anIndex ? MyQuestionerId() : QuestionerId()),
+    index_(anIndex)
+{
+}
+
+std::ostream &
+Ipc::operator <<(std::ostream &os, const RequestId &requestId)
+{
+    os << requestId.index() << '@' << requestId.questioner();
+    return os;
+}
+
diff --git a/src/ipc/RequestId.h b/src/ipc/RequestId.h
new file mode 100644 (file)
index 0000000..6427034
--- /dev/null
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 1996-2020 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_IPC_REQUESTID_H
+#define SQUID_IPC_REQUESTID_H
+
+#include "ipc/forward.h"
+#include "ipc/QuestionerId.h"
+
+#include <iosfwd>
+
+namespace Ipc
+{
+
+/// uniquely identifies an IPC request among same-type concurrent IPC requests
+/// submitted by a single Squid instance
+class RequestId
+{
+public:
+    /// A simple ID for correlating IPC responses with pending requests.
+    /// Value 0 has a special meaning of "unset/unknown", but otherwise opaque.
+    typedef unsigned int Index;
+
+    /// Request sender's constructor.
+    /// For performance and clarity sake, default constructor is preferred to 0 index.
+    explicit RequestId(Index);
+
+    /// request recipient's constructor
+    RequestId() = default;
+
+    /// Make the ID unset/unknown.
+    /// Optimization: leaves the questioner field alone.
+    void reset() { index_ = 0; }
+
+    /// Make the ID set/known with the given (by the questioner) index.
+    /// For performance and clarity sake, reset(void) is preferred to reset(0).
+    void reset(const Index anIndex) { *this = RequestId(anIndex); }
+
+    QuestionerId questioner() const { return qid_; }
+    Index index() const { return index_; }
+
+    // these conversion operators allow our users to treat us as an Index
+    operator Index() const { return index_; }
+    RequestId &operator =(const Index anIndex) { anIndex ? reset(anIndex) : reset(); return *this; }
+
+private:
+    /// the sender of the request
+    QuestionerId qid_;
+
+    /// request ID; unique within pending same-qid_ questions of the same kind
+    Index index_ = 0;
+};
+
+std::ostream &operator <<(std::ostream &, const RequestId &);
+
+} // namespace Ipc;
+
+#endif /* SQUID_IPC_REQUESTID_H */
+
index 76ba916507a2ffb2a66b6a78a892121c192d0178..ce278167df943b06937aa7835957a4a2323ea6fd 100644 (file)
 #define SQUID_IPC_RESPONSE_H
 
 #include "base/RefCount.h"
+#include "base/TypeTraits.h"
 #include "ipc/forward.h"
+#include "ipc/QuestionerId.h"
 
 namespace Ipc
 {
 
 /// A response to Ipc::Request.
-class Response: public RefCountable
+class Response: public RefCountable, public Interface
 {
 public:
     typedef RefCount<Response> Pointer;
 
 public:
-    explicit Response(unsigned int aRequestId):
-        requestId(aRequestId) {}
-
     virtual void pack(TypedMsgHdr& msg) const = 0; ///< prepare for sendmsg()
     virtual Pointer clone() const = 0; ///< returns a copy of this
 
-private:
-    Response(const Response&); // not implemented
-    Response& operator= (const Response&); // not implemented
+    /// for Mine() tests
+    QuestionerId intendedRecepient() const { return requestId.questioner(); }
 
 public:
-    unsigned int requestId; ///< ID of request we are responding to
-};
+    RequestId requestId; ///< the ID of the request we are responding to
 
-inline
-std::ostream& operator << (std::ostream &os, const Response& response)
-{
-    os << "[response.requestId %u]" << response.requestId << '}';
-    return os;
-}
+protected:
+    /// sender's constructor
+    explicit Response(const RequestId aRequestId): requestId(aRequestId) {}
+
+    /// recipient's constructor
+    Response() = default;
+};
 
 } // namespace Ipc
 
index 60e9e358017ef286cfbccf767e1a8c005dc18a0c..d873bf26ce5340e964f7c1b7777a573ab2d0d27c 100644 (file)
@@ -33,25 +33,26 @@ public:
 };
 
 /// maps ID assigned at request time to the response callback
-typedef std::map<int, PendingOpenRequest> SharedListenRequestMap;
+typedef std::map<Ipc::RequestId::Index, PendingOpenRequest> SharedListenRequestMap;
 static SharedListenRequestMap TheSharedListenRequestMap;
 
 /// accumulates delayed requests until they are ready to be sent, in FIFO order
 typedef std::list<PendingOpenRequest> DelayedSharedListenRequests;
 static DelayedSharedListenRequests TheDelayedRequests;
 
-static int
+// TODO: Encapsulate "Pending Request Map" logic shared by all RequestId users.
+/// registers the given request in the collection of pending requests
+/// \returns the registration key
+static Ipc::RequestId::Index
 AddToMap(const PendingOpenRequest &por)
 {
-    // find unused ID using linear search; there should not be many entries
-    for (int id = 0; true; ++id) {
-        if (TheSharedListenRequestMap.find(id) == TheSharedListenRequestMap.end()) {
-            TheSharedListenRequestMap[id] = por;
-            return id;
-        }
-    }
-    assert(false); // not reached
-    return -1;
+    static Ipc::RequestId::Index LastIndex = 0;
+    // TODO: Switch Ipc::RequestId::Index to uint64_t and drop these 0 checks.
+    if (++LastIndex == 0) // don't use zero value as an ID
+        ++LastIndex;
+    assert(TheSharedListenRequestMap.find(LastIndex) == TheSharedListenRequestMap.end());
+    TheSharedListenRequestMap[LastIndex] = por;
+    return LastIndex;
 }
 
 bool
@@ -68,7 +69,10 @@ Ipc::OpenListenerParams::operator <(const OpenListenerParams &p) const
     return addr.compareWhole(p.addr) < 0;
 }
 
-Ipc::SharedListenRequest::SharedListenRequest(): requestorId(-1), mapId(-1)
+Ipc::SharedListenRequest::SharedListenRequest(const OpenListenerParams &aParams, const RequestId aMapId):
+    requestorId(KidIdentifier),
+    params(aParams),
+    mapId(aMapId)
 {
     // caller will then set public data members
 }
@@ -76,22 +80,25 @@ Ipc::SharedListenRequest::SharedListenRequest(): requestorId(-1), mapId(-1)
 Ipc::SharedListenRequest::SharedListenRequest(const TypedMsgHdr &hdrMsg)
 {
     hdrMsg.checkType(mtSharedListenRequest);
+    // XXX: our handlerSubscription is not a POD!
     hdrMsg.getPod(*this);
 }
 
 void Ipc::SharedListenRequest::pack(TypedMsgHdr &hdrMsg) const
 {
     hdrMsg.setType(mtSharedListenRequest);
+    // XXX: our handlerSubscription is not a POD!
     hdrMsg.putPod(*this);
 }
 
-Ipc::SharedListenResponse::SharedListenResponse(int aFd, int anErrNo, int aMapId):
+Ipc::SharedListenResponse::SharedListenResponse(const int aFd, const int anErrNo, const RequestId aMapId):
     fd(aFd), errNo(anErrNo), mapId(aMapId)
 {
 }
 
 Ipc::SharedListenResponse::SharedListenResponse(const TypedMsgHdr &hdrMsg):
-    fd(-1), errNo(0), mapId(-1)
+    fd(-1),
+    errNo(0)
 {
     hdrMsg.checkType(mtSharedListenResponse);
     hdrMsg.getPod(*this);
@@ -103,16 +110,14 @@ void Ipc::SharedListenResponse::pack(TypedMsgHdr &hdrMsg) const
 {
     hdrMsg.setType(mtSharedListenResponse);
     hdrMsg.putPod(*this);
+    // XXX: When we respond with an error, putFd() throws due to the negative fd
     hdrMsg.putFd(fd);
 }
 
 static void
 SendSharedListenRequest(const PendingOpenRequest &por)
 {
-    Ipc::SharedListenRequest request;
-    request.requestorId = KidIdentifier;
-    request.params = por.params;
-    request.mapId = AddToMap(por);
+    const Ipc::SharedListenRequest request(por.params, Ipc::RequestId(AddToMap(por)));
 
     debugs(54, 3, "getting listening FD for " << request.params.addr <<
            " mapId=" << request.mapId);
@@ -160,10 +165,12 @@ void Ipc::SharedListenJoined(const SharedListenResponse &response)
            TheSharedListenRequestMap.size() << " active + " <<
            TheDelayedRequests.size() << " delayed requests");
 
-    Must(TheSharedListenRequestMap.find(response.mapId) != TheSharedListenRequestMap.end());
-    PendingOpenRequest por = TheSharedListenRequestMap[response.mapId];
-    Must(por.callback != NULL);
-    TheSharedListenRequestMap.erase(response.mapId);
+    Must(response.mapId);
+    const auto pori = TheSharedListenRequestMap.find(response.mapId.index());
+    Must(pori != TheSharedListenRequestMap.end());
+    auto por = pori->second;
+    Must(por.callback);
+    TheSharedListenRequestMap.erase(pori);
 
     StartListeningCb *cbd = dynamic_cast<StartListeningCb*>(por.callback->getDialer());
     assert(cbd && cbd->conn != NULL);
index 13a0700496e95391d62a688a4053cbfa63865f66..a6a7059b1671ac3a12eebd4e375792be43765df7 100644 (file)
@@ -13,6 +13,8 @@
 
 #include "base/AsyncCall.h"
 #include "base/Subscription.h"
+#include "ipc/QuestionerId.h"
+#include "ipc/RequestId.h"
 
 namespace Ipc
 {
@@ -45,7 +47,7 @@ class TypedMsgHdr;
 class SharedListenRequest
 {
 public:
-    SharedListenRequest(); ///< from OpenSharedListen() which then sets public data
+    SharedListenRequest(const OpenListenerParams &, RequestId aMapId); ///< sender's constructor
     explicit SharedListenRequest(const TypedMsgHdr &hdrMsg); ///< from recvmsg()
     void pack(TypedMsgHdr &hdrMsg) const; ///< prepare for sendmsg()
 
@@ -54,21 +56,24 @@ public:
 
     OpenListenerParams params; ///< actual comm_open_sharedListen() parameters
 
-    int mapId; ///< to map future response to the requestor's callback
+    RequestId mapId; ///< to map future response to the requestor's callback
 };
 
 /// a response to SharedListenRequest
 class SharedListenResponse
 {
 public:
-    SharedListenResponse(int fd, int errNo, int mapId);
+    SharedListenResponse(int fd, int errNo, RequestId aMapId); ///< sender's constructor
     explicit SharedListenResponse(const TypedMsgHdr &hdrMsg); ///< from recvmsg()
     void pack(TypedMsgHdr &hdrMsg) const; ///< prepare for sendmsg()
 
+    /// for Mine() tests
+    QuestionerId intendedRecepient() const { return mapId.questioner(); }
+
 public:
     int fd; ///< opened listening socket or -1
     int errNo; ///< errno value from comm_open_sharedListen() call
-    int mapId; ///< to map future response to the requestor's callback
+    RequestId mapId; ///< to map future response to the requestor's callback
 };
 
 /// prepare and send SharedListenRequest to Coordinator
index 6d844e9323feebd02d4dee4a6cce3bd0b67a4249..240a2aa21c988810415001f1996af072436acf94 100644 (file)
@@ -18,6 +18,7 @@
 #include "globals.h"
 #include "ipc/Kids.h"
 #include "ipc/Messages.h"
+#include "ipc/QuestionerId.h"
 #include "ipc/SharedListen.h"
 #include "ipc/Strand.h"
 #include "ipc/StrandCoord.h"
@@ -62,16 +63,16 @@ void Ipc::Strand::receive(const TypedMsgHdr &message)
     switch (message.rawType()) {
 
     case mtStrandRegistered:
-        handleRegistrationResponse(StrandMessage(message));
+        handleRegistrationResponse(Mine(StrandMessage(message)));
         break;
 
     case mtSharedListenResponse:
-        SharedListenJoined(SharedListenResponse(message));
+        SharedListenJoined(Mine(SharedListenResponse(message)));
         break;
 
 #if HAVE_DISKIO_MODULE_IPCIO
     case mtStrandReady:
-        IpcIoFile::HandleOpenResponse(StrandMessage(message));
+        IpcIoFile::HandleOpenResponse(Mine(StrandMessage(message)));
         break;
 
     case mtIpcIoNotification:
@@ -87,7 +88,7 @@ void Ipc::Strand::receive(const TypedMsgHdr &message)
 
     case mtCacheMgrResponse: {
         const Mgr::Response resp(message);
-        handleCacheMgrResponse(resp);
+        handleCacheMgrResponse(Mine(resp));
     }
     break;
 
@@ -104,13 +105,13 @@ void Ipc::Strand::receive(const TypedMsgHdr &message)
 
     case mtSnmpResponse: {
         const Snmp::Response resp(message);
-        handleSnmpResponse(resp);
+        handleSnmpResponse(Mine(resp));
     }
     break;
 #endif
 
     default:
-        debugs(54, DBG_IMPORTANT, "WARNING: Ignoring IPC message with an unknown type: " << message.rawType());
+        Port::receive(message);
         break;
     }
 }
index fc35b0fb86c1300f0da7bb13d74a174505983613..df299cf075d4b9841adda08bc149bbc59c678334 100644 (file)
@@ -38,14 +38,16 @@ void Ipc::StrandCoord::pack(TypedMsgHdr &hdrMsg) const
     hdrMsg.putString(tag);
 }
 
-Ipc::StrandMessage::StrandMessage(const StrandCoord &aStrand):
-    strand(aStrand)
+Ipc::StrandMessage::StrandMessage(const StrandCoord &aStrand, const QuestionerId aQid):
+    strand(aStrand),
+    qid(aQid)
 {
 }
 
 Ipc::StrandMessage::StrandMessage(const TypedMsgHdr &hdrMsg)
 {
     strand.unpack(hdrMsg);
+    qid.unpack(hdrMsg);
 }
 
 void
@@ -53,13 +55,14 @@ Ipc::StrandMessage::pack(const MessageType messageType, TypedMsgHdr &hdrMsg) con
 {
     hdrMsg.setType(messageType);
     strand.pack(hdrMsg);
+    qid.pack(hdrMsg);
 }
 
 void
 Ipc::StrandMessage::NotifyCoordinator(const MessageType msgType, const char *tag)
 {
     static const auto pid = getpid();
-    StrandMessage message(StrandCoord(KidIdentifier, pid));
+    StrandMessage message(StrandCoord(KidIdentifier, pid), MyQuestionerId());
     if (tag)
         message.strand.tag = tag;
     TypedMsgHdr hdr;
index 2758d7777c4d24e7820938c4b141b17942e1043f..1778f0d55ab89f26c6c6c4dcf7fdcfde23ae96be 100644 (file)
@@ -11,6 +11,7 @@
 
 #include "ipc/forward.h"
 #include "ipc/Messages.h"
+#include "ipc/QuestionerId.h"
 #include "SquidString.h"
 
 namespace Ipc
@@ -37,15 +38,22 @@ public:
 class StrandMessage
 {
 public:
-    explicit StrandMessage(const StrandCoord &);
+    explicit StrandMessage(const StrandCoord &, QuestionerId);
     explicit StrandMessage(const TypedMsgHdr &);
     void pack(MessageType, TypedMsgHdr &) const;
 
     /// creates and sends StrandMessage to Coordinator
     static void NotifyCoordinator(MessageType, const char *tag);
 
+    /// for Mine() tests
+    QuestionerId intendedRecepient() const { return qid; }
+
 public:
     StrandCoord strand; ///< messageType-specific coordinates (e.g., sender)
+
+    /// For IPC requests/questions: The sender of this request.
+    /// For IPC responses/answers: The sender of the corresponding request.
+    QuestionerId qid;
 };
 
 } // namespace Ipc;
index 13f9406ebbd032bdacd57a60d604a0d52ed49f53..f75fddaada27d88295b4efd25350b7099b594261 100644 (file)
@@ -9,11 +9,15 @@
 /* DEBUG: section 54    Interprocess Communication */
 
 #include "squid.h"
+#include "globals.h"
 #include "ipc/Messages.h"
 #include "ipc/StrandSearch.h"
 #include "ipc/TypedMsgHdr.h"
 
-Ipc::StrandSearchRequest::StrandSearchRequest(): requestorId(-1)
+Ipc::StrandSearchRequest::StrandSearchRequest(const String &aTag):
+    requestorId(KidIdentifier),
+    tag(aTag),
+    qid(MyQuestionerId())
 {
 }
 
@@ -23,6 +27,7 @@ Ipc::StrandSearchRequest::StrandSearchRequest(const TypedMsgHdr &hdrMsg):
     hdrMsg.checkType(mtFindStrand);
     hdrMsg.getPod(requestorId);
     hdrMsg.getString(tag);
+    qid.unpack(hdrMsg);
 }
 
 void Ipc::StrandSearchRequest::pack(TypedMsgHdr &hdrMsg) const
@@ -30,5 +35,6 @@ void Ipc::StrandSearchRequest::pack(TypedMsgHdr &hdrMsg) const
     hdrMsg.setType(mtFindStrand);
     hdrMsg.putPod(requestorId);
     hdrMsg.putString(tag);
+    qid.pack(hdrMsg);
 }
 
index 3c3cd9d8cf8a44b26dd04f85fe1c27d3f9eadfeb..ad4cb44953f28c387306b5251923169630e63da5 100644 (file)
@@ -10,6 +10,7 @@
 #define SQUID_IPC_STRAND_SEARCH_H
 
 #include "ipc/forward.h"
+#include "ipc/QuestionerId.h"
 #include "ipc/StrandCoord.h"
 #include "SquidString.h"
 
@@ -20,13 +21,14 @@ namespace Ipc
 class StrandSearchRequest
 {
 public:
-    StrandSearchRequest();
+    explicit StrandSearchRequest(const String &aTag); ///< sender's constructor
     explicit StrandSearchRequest(const TypedMsgHdr &hdrMsg); ///< from recvmsg()
     void pack(TypedMsgHdr &hdrMsg) const; ///< prepare for sendmsg()
 
 public:
     int requestorId; ///< sender-provided return address
     String tag; ///< set when looking for a matching StrandCoord::tag
+    QuestionerId qid; ///< the sender of the request
 };
 
 } // namespace Ipc;
index 5fc7ac443fac9b1901f0111901bf60f41e2a08fe..4953fa4e4295e349801db96641120b11d6b9c7e7 100644 (file)
@@ -48,11 +48,9 @@ public:
     /// \returns 0 if no message kind has been received or set
     int rawType() const { return msg_iov ? data.type_ : 0; }
 
-    /* access for Plain Old Data (POD)-based message parts */
-    template <class Pod>
-    void getPod(Pod &pod) const { getFixed(&pod, sizeof(pod)); } ///< load POD
-    template <class Pod>
-    void putPod(const Pod &pod) { putFixed(&pod, sizeof(pod)); } ///< store POD
+    /* access for TriviallyCopyable (a.k.a. Plain Old Data or POD) message parts */
+    template <class Pod> void getPod(Pod &pod) const; ///< load POD
+    template <class Pod> void putPod(const Pod &pod); ///< store POD
 
     /* access to message parts for selected commonly-used part types */
     void getString(String &s) const; ///< load variable-length string
@@ -113,5 +111,23 @@ private:
 
 } // namespace Ipc
 
+template <class Pod>
+void
+Ipc::TypedMsgHdr::getPod(Pod &pod) const
+{
+    // TODO: Enable after fixing Ipc::SharedListenRequest::SharedListenRequest()
+    //static_assert(std::is_trivially_copyable<Pod>::value, "getPod() used for a POD");
+    getFixed(&pod, sizeof(pod));
+}
+
+template <class Pod>
+void
+Ipc::TypedMsgHdr::putPod(const Pod &pod)
+{
+    // TODO: Enable after fixing Ipc::SharedListenRequest::pack()
+    //static_assert(std::is_trivially_copyable<Pod>::value, "putPod() used for a POD");
+    putFixed(&pod, sizeof(pod));
+}
+
 #endif /* SQUID_IPC_TYPED_MSG_HDR_H */
 
index 8fff7804a122aa67733b5882e2ad3b22fd9a0d94..cee2b9478c510868e5f4ecd9fad3a17960a815b2 100644 (file)
 namespace Ipc
 {
 
-class TypedMsgHdr;
-class StrandCoord;
-class StrandMessage;
 class Forwarder;
 class Inquirer;
+class QuestionerId;
 class Request;
+class RequestId;
 class Response;
+class StrandCoord;
+class StrandMessage;
+class TypedMsgHdr;
 
 } // namespace Ipc
 
index 343dceb49b13dc7db28d9f148dd2001de6c5b69f..2ca5eef1e675794e64228cf48b0fd42198c79719 100644 (file)
@@ -79,7 +79,7 @@ Mgr::Action::respond(const Request &request)
 }
 
 void
-Mgr::Action::sendResponse(unsigned int requestId)
+Mgr::Action::sendResponse(const Ipc::RequestId requestId)
 {
     Response response(requestId, this);
     Ipc::TypedMsgHdr message;
index 0efcd12c5ee19cbaa7d80aafbdeda43aaa0fc72e..6ccee5b64541820579ee58f10b674faf9b65fcb9 100644 (file)
@@ -55,7 +55,7 @@ public:
     virtual void unpack(const Ipc::TypedMsgHdr &) {}
 
     /// notify Coordinator that this action is done with local processing
-    void sendResponse(unsigned int requestId);
+    void sendResponse(Ipc::RequestId);
 
     /* Action properties */
 
index bf50e47df2e7e6ff889bd6ee98f8c2eb06e8dab8..4d4a2b0bcef9b938abce7d3d866691bf6b422a02 100644 (file)
@@ -11,6 +11,7 @@
 #include "squid.h"
 #include "base/TextException.h"
 #include "comm/Connection.h"
+#include "ipc/RequestId.h"
 #include "mgr/Filler.h"
 #include "mgr/Response.h"
 #include "Store.h"
@@ -18,7 +19,7 @@
 CBDATA_NAMESPACED_CLASS_INIT(Mgr, Filler);
 
 Mgr::Filler::Filler(const Action::Pointer &anAction, const Comm::ConnectionPointer &conn,
-                    unsigned int aRequestId):
+                    const Ipc::RequestId aRequestId):
     StoreToCommWriter(conn, anAction->createStoreEntry()),
     action(anAction),
     requestId(aRequestId)
index 26b5f01e15d6a2c8df80c93f80a0e803aa937298..f9f3d3ea1a8b53689d7a4934c4538033a5fa1739 100644 (file)
@@ -12,6 +12,7 @@
 #define SQUID_MGR_FILLER_H
 
 #include "comm/forward.h"
+#include "ipc/forward.h"
 #include "mgr/Action.h"
 #include "mgr/StoreToCommWriter.h"
 
@@ -24,7 +25,7 @@ class Filler: public StoreToCommWriter
     CBDATA_CLASS(Filler);
 
 public:
-    Filler(const Action::Pointer &anAction, const Comm::ConnectionPointer &conn, unsigned int aRequestId);
+    Filler(const Action::Pointer &, const Comm::ConnectionPointer &, Ipc::RequestId);
 
 protected:
     /* AsyncJob API */
@@ -33,7 +34,7 @@ protected:
 
 private:
     Action::Pointer action; ///< action that will run() and sendResponse()
-    unsigned int requestId; ///< the ID of the Request we are responding to
+    Ipc::RequestId requestId; ///< the ID of the Request we are responding to
 };
 
 } // namespace Mgr
index 0486d01ce6fd5d7aff53a4ceb477eaf2d683046e..aefbae3c8e6b561a0f20ccd0f5e2e15be14830fe 100644 (file)
@@ -28,7 +28,10 @@ CBDATA_NAMESPACED_CLASS_INIT(Mgr, Forwarder);
 
 Mgr::Forwarder::Forwarder(const Comm::ConnectionPointer &aConn, const ActionParams &aParams,
                           HttpRequest* aRequest, StoreEntry* anEntry, const AccessLogEntryPointer &anAle):
-    Ipc::Forwarder(new Request(KidIdentifier, 0, aConn, aParams), 10),
+    // TODO: Add virtual Forwarder::makeRequest() to avoid prematurely creating
+    // this dummy request with a dummy ID that are finalized by Ipc::Forwarder.
+    // Same for Snmp::Forwarder.
+    Ipc::Forwarder(new Request(KidIdentifier, Ipc::RequestId(/*XXX*/), aConn, aParams), 10),
     httpRequest(aRequest), entry(anEntry), conn(aConn), ale(anAle)
 {
     debugs(16, 5, HERE << conn);
index ccba1e6e7266f77fb42fdca604dc828443296419..a4ca76dd7ae3c9261896989da1d276f8099784af 100644 (file)
@@ -12,6 +12,7 @@
 #include "base/TextException.h"
 #include "comm/Connection.h"
 #include "globals.h"
+#include "ipc/RequestId.h"
 #include "ipc/UdsOp.h"
 #include "mgr/Command.h"
 #include "mgr/Filler.h"
index 49fe61d118412beb2b4e43ce13a912f2104fd56f..87968b4ea5c90d5fce6a5261e79e230b409e1016 100644 (file)
@@ -14,6 +14,7 @@
 #include "globals.h"
 #include "HttpReply.h"
 #include "ipc/Messages.h"
+#include "ipc/RequestId.h"
 #include "ipc/TypedMsgHdr.h"
 #include "ipc/UdsOp.h"
 #include "mgr/Filler.h"
index 8b9dfe9be62fe014d6955d38834f17ea8a32dded..d1d5fc53cf95a6d0fc0db868cbc9b818e797d9b9 100644 (file)
@@ -16,7 +16,9 @@
 #include "mgr/ActionParams.h"
 #include "mgr/Request.h"
 
-Mgr::Request::Request(int aRequestorId, unsigned int aRequestId, const Comm::ConnectionPointer &aConn,
+Mgr::Request::Request(const int aRequestorId,
+                      const Ipc::RequestId aRequestId,
+                      const Comm::ConnectionPointer &aConn,
                       const ActionParams &aParams):
     Ipc::Request(aRequestorId, aRequestId),
     conn(aConn),
@@ -25,14 +27,7 @@ Mgr::Request::Request(int aRequestorId, unsigned int aRequestId, const Comm::Con
     Must(requestorId > 0);
 }
 
-Mgr::Request::Request(const Request& request):
-    Ipc::Request(request.requestorId, request.requestId),
-    conn(request.conn), params(request.params)
-{
-}
-
-Mgr::Request::Request(const Ipc::TypedMsgHdr& msg):
-    Ipc::Request(0, 0)
+Mgr::Request::Request(const Ipc::TypedMsgHdr &msg)
 {
     msg.checkType(Ipc::mtCacheMgrRequest);
     msg.getPod(requestorId);
index fce310cc36fa3ae36b4f10dda456b6310333f400..9399e37232375cb6134af8aad8eb1b43b83a697b 100644 (file)
@@ -22,7 +22,7 @@ namespace Mgr
 class Request: public Ipc::Request
 {
 public:
-    Request(int aRequestorId, unsigned int aRequestId, const Comm::ConnectionPointer &aConn,
+    Request(int aRequestorId, Ipc::RequestId, const Comm::ConnectionPointer &aConn,
             const ActionParams &aParams);
 
     explicit Request(const Ipc::TypedMsgHdr& msg); ///< from recvmsg()
@@ -30,9 +30,6 @@ public:
     virtual void pack(Ipc::TypedMsgHdr& msg) const;
     virtual Pointer clone() const;
 
-private:
-    Request(const Request& request);
-
 public:
     Comm::ConnectionPointer conn; ///< HTTP client connection descriptor
 
index 8d40a37d70c632d24dcda71333ea65b4dd9437ca..37cab587a593bb4e926221d24a2e92ccd156e12d 100644 (file)
 #include "base/TextException.h"
 #include "CacheManager.h"
 #include "ipc/Messages.h"
+#include "ipc/RequestId.h"
 #include "ipc/TypedMsgHdr.h"
 #include "mgr/ActionCreator.h"
 #include "mgr/ActionProfile.h"
 #include "mgr/Response.h"
 
-Mgr::Response::Response(unsigned int aRequestId, Action::Pointer anAction):
+Mgr::Response::Response(const Ipc::RequestId aRequestId, const Action::Pointer anAction):
     Ipc::Response(aRequestId), action(anAction)
 {
     Must(!action || action->name()); // if there is an action, it must be named
 }
 
-Mgr::Response::Response(const Response& response):
-    Ipc::Response(response.requestId), action(response.action)
-{
-}
-
-Mgr::Response::Response(const Ipc::TypedMsgHdr& msg):
-    Ipc::Response(0)
+Mgr::Response::Response(const Ipc::TypedMsgHdr &msg)
 {
     msg.checkType(Ipc::mtCacheMgrResponse);
     msg.getPod(requestId);
index e95a6d5b45b50c4baa715323eee0a20801148548..13e4c189bc3b507bf49f50cf09389ae593df75b9 100644 (file)
@@ -23,7 +23,8 @@ namespace Mgr
 class Response: public Ipc::Response
 {
 public:
-    Response(unsigned int aRequestId, Action::Pointer anAction = NULL);
+    /// sender's constructor
+    Response(Ipc::RequestId, Action::Pointer anAction = nullptr);
 
     explicit Response(const Ipc::TypedMsgHdr& msg); ///< from recvmsg()
 
@@ -34,9 +35,6 @@ public:
     bool hasAction() const; ///< whether response contain action object
     const Action& getAction() const; ///< returns action object
 
-private:
-    Response(const Response& response);
-
 public:
     Action::Pointer action; ///< action relating to response
 };
index 51b2932272e8059871276a988bfdc339e1ed22a0..6fd3bd8a06dabf4a5fd039b929d9a2e4ec7119bf 100644 (file)
@@ -23,7 +23,7 @@ CBDATA_NAMESPACED_CLASS_INIT(Snmp, Forwarder);
 
 Snmp::Forwarder::Forwarder(const Pdu& aPdu, const Session& aSession, int aFd,
                            const Ip::Address& anAddress):
-    Ipc::Forwarder(new Request(KidIdentifier, 0, aPdu, aSession, aFd, anAddress), 2),
+    Ipc::Forwarder(new Request(KidIdentifier, Ipc::RequestId(), aPdu, aSession, aFd, anAddress), 2),
     fd(aFd)
 {
     debugs(49, 5, HERE << "FD " << aFd);
@@ -88,7 +88,7 @@ Snmp::Forwarder::sendError(int error)
 }
 
 void
-Snmp::SendResponse(unsigned int requestId, const Pdu& pdu)
+Snmp::SendResponse(const Ipc::RequestId requestId, const Pdu &pdu)
 {
     debugs(49, 5, HERE);
     // snmpAgentResponse() can modify arg
index a8c7560af6b3a2c39998ae406a3e09f443f9316f..0eecd6e1aca037ee80e714cd34feae84981596b9 100644 (file)
@@ -47,7 +47,7 @@ private:
     AsyncCall::Pointer closer; ///< comm_close handler for the connection
 };
 
-void SendResponse(unsigned int requestId, const Pdu& pdu);
+void SendResponse(Ipc::RequestId, const Pdu &);
 
 } // namespace Snmp
 
index e5247a971e35711e39c6f226c6e4df106e20253e..750e491cf80b4ebb11d97bd2bfe8310b5b4a2f41 100644 (file)
@@ -13,7 +13,7 @@
 #include "ipc/TypedMsgHdr.h"
 #include "snmp/Request.h"
 
-Snmp::Request::Request(int aRequestorId, unsigned int aRequestId,
+Snmp::Request::Request(const int aRequestorId, const Ipc::RequestId aRequestId,
                        const Pdu& aPdu, const Session& aSession,
                        int aFd, const Ip::Address& anAddress):
     Ipc::Request(aRequestorId, aRequestId),
@@ -21,15 +21,7 @@ Snmp::Request::Request(int aRequestorId, unsigned int aRequestId,
 {
 }
 
-Snmp::Request::Request(const Request& request):
-    Ipc::Request(request.requestorId, request.requestId),
-    pdu(request.pdu), session(request.session),
-    fd(request.fd), address(request.address)
-{
-}
-
-Snmp::Request::Request(const Ipc::TypedMsgHdr& msg):
-    Ipc::Request(0, 0)
+Snmp::Request::Request(const Ipc::TypedMsgHdr &msg)
 {
     msg.checkType(Ipc::mtSnmpRequest);
     msg.getPod(requestorId);
index 739dda6819e3f6c41e58f490d697314e5e8d011b..df9adbe6051bb90bff0606b0651a62ae296bb688 100644 (file)
@@ -24,7 +24,7 @@ namespace Snmp
 class Request: public Ipc::Request
 {
 public:
-    Request(int aRequestorId, unsigned int aRequestId, const Pdu& aPdu,
+    Request(int aRequestorId, Ipc::RequestId aRequestId, const Pdu& aPdu,
             const Session& aSession, int aFd, const Ip::Address& anAddress);
 
     explicit Request(const Ipc::TypedMsgHdr& msg); ///< from recvmsg()
@@ -32,9 +32,6 @@ public:
     virtual void pack(Ipc::TypedMsgHdr& msg) const;
     virtual Pointer clone() const;
 
-private:
-    Request(const Request& request);
-
 public:
     Pdu pdu; ///< SNMP protocol data unit
     Session session; ///< SNMP session
index 65495d45c8939658a148931ec63c717fb9f66976..c7f522945bb2f9436491e99ee0fd616a46b70ded 100644 (file)
 #include "squid.h"
 #include "base/TextException.h"
 #include "ipc/Messages.h"
+#include "ipc/RequestId.h"
 #include "ipc/TypedMsgHdr.h"
 #include "snmp/Response.h"
 
-std::ostream& Snmp::operator << (std::ostream& os, const Response& response)
-{
-    os << "response: {requestId: " << response.requestId << '}';
-    return os;
-}
-
-Snmp::Response::Response(unsigned int aRequestId):
+Snmp::Response::Response(const Ipc::RequestId aRequestId):
     Ipc::Response(aRequestId), pdu()
 {
 }
 
-Snmp::Response::Response(const Response& response):
-    Ipc::Response(response.requestId), pdu(response.pdu)
-{
-}
-
-Snmp::Response::Response(const Ipc::TypedMsgHdr& msg):
-    Ipc::Response(0)
+Snmp::Response::Response(const Ipc::TypedMsgHdr &msg)
 {
     msg.checkType(Ipc::mtSnmpResponse);
     msg.getPod(requestId);
index 7f787011c87fcc1aea988cd31a40c3f3ff44eb2d..0845a2d17b07f744dc8873058156e4a91e091243 100644 (file)
@@ -23,21 +23,16 @@ namespace Snmp
 class Response: public Ipc::Response
 {
 public:
-    Response(unsigned int aRequestId);
+    explicit Response(Ipc::RequestId); ///< sender's constructor
     explicit Response(const Ipc::TypedMsgHdr& msg); ///< from recvmsg()
     /* Ipc::Response API */
     virtual void pack(Ipc::TypedMsgHdr& msg) const;
     virtual Ipc::Response::Pointer clone() const;
 
-private:
-    Response(const Response& response);
-
 public:
     Pdu pdu; ///< SNMP protocol data unit
 };
 
-std::ostream& operator << (std::ostream& os, const Response& response);
-
 } // namespace Snmp
 
 #endif /* SQUID_SNMPX_RESPONSE_H */
index 742da1ea54993b58f1a863930043ff5463d8d866..b2a1335b78509a416d739c28ba41e52fa2992c95 100644 (file)
@@ -12,6 +12,8 @@
 #define STUB_API "lmgr/libmgr.la"
 #include "tests/STUB.h"
 
+#include "ipc/RequestId.h"
+
 // NP: used by Command.h instantiations
 #include "mgr/ActionProfile.h"
 
@@ -26,7 +28,7 @@ void Mgr::Action::run(StoreEntry *entry, bool writeHttpHeader) STUB
 void Mgr::Action::fillEntry(StoreEntry *entry, bool writeHttpHeader) STUB
 void Mgr::Action::add(const Action &action) STUB
 void Mgr::Action::respond(const Request &request) STUB
-void Mgr::Action::sendResponse(unsigned int requestId) STUB
+void Mgr::Action::sendResponse(const Ipc::RequestId) STUB
 bool Mgr::Action::atomic() const STUB_RETVAL(false)
 const char * Mgr::Action::name() const STUB_RETVAL(NULL)
 static Mgr::Command static_Command;