From b248c2a378826c9c027192b5db4fabd984fd7b13 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Wed, 13 Feb 2013 00:34:35 +1300 Subject: [PATCH] Polish: Drop redundant HttpMsgPointerT template Despite the object and API changes being rather extensive there are no logic changes in this patch. The HttpMsgPointerT template was used to ref-count HttpMsg objects and its children. It provided an API identical to the RefCount API plus a few extensions for handling polymorphism cast operations in the background. This patch: * update HttpMsg class hierarchy to use Lock API directly removing the _lock() and _unlock() virtual functions. * update HttpControlMsg to use a HttpReply::Pointer directly since this type is available to that class a re-definition using HttpMsg* casting was not needed. * update HTTPMSGLOCK() macro not to return a HttpMsg* The API polymorphism extensions provided by HttpMsgPointerT<> were only necessary to handle the HttpMsg* object casting this macro made a requirement. Once that assignment casting is removed the entire API extensions are no longer used. --- src/HttpControlMsg.h | 5 +-- src/HttpMsg.cc | 16 -------- src/HttpMsg.h | 66 ++----------------------------- src/HttpReply.cc | 6 ++- src/HttpReply.h | 11 +----- src/HttpRequest.h | 7 +--- src/MemObject.cc | 7 ++-- src/Notes.cc | 3 +- src/Server.cc | 11 ++++-- src/acl/Asn.cc | 7 ++-- src/acl/FilledChecklist.cc | 3 +- src/adaptation/AccessCheck.cc | 3 +- src/adaptation/Answer.h | 2 +- src/adaptation/Iterator.cc | 14 +++++-- src/adaptation/Message.cc | 3 +- src/adaptation/ServiceFilter.cc | 32 ++++++++++----- src/adaptation/icap/InOut.h | 6 ++- src/adaptation/icap/Launcher.cc | 25 ++++++++---- src/adaptation/icap/ModXact.cc | 25 ++++++------ src/adaptation/icap/OptXact.cc | 6 +-- src/adaptation/icap/ServiceRep.cc | 4 +- src/adaptation/icap/Xaction.cc | 13 +++--- src/client_side.cc | 66 ++++++++++++++++++------------- src/client_side_reply.cc | 14 ++++--- src/client_side_request.cc | 17 +++++--- src/errorpage.cc | 5 ++- src/errorpage.h | 2 +- src/forward.cc | 12 ++++-- src/http.cc | 5 ++- src/icp_v2.cc | 6 ++- src/mime.cc | 3 +- src/neighbors.cc | 6 ++- src/peer_digest.cc | 9 +++-- src/peer_select.cc | 3 +- src/ssl/ErrorDetail.cc | 2 +- src/ssl/ErrorDetailManager.cc | 6 +-- src/ssl/ErrorDetailManager.h | 2 +- src/ssl/ServerBump.cc | 2 +- src/store_digest.cc | 3 +- src/tests/stub_errorpage.cc | 2 +- src/tunnel.cc | 3 +- src/urn.cc | 3 +- 42 files changed, 220 insertions(+), 226 deletions(-) diff --git a/src/HttpControlMsg.h b/src/HttpControlMsg.h index 1bfbf90f15..667329446c 100644 --- a/src/HttpControlMsg.h +++ b/src/HttpControlMsg.h @@ -29,14 +29,13 @@ public: class HttpControlMsg { public: - typedef HttpMsgPointerT MsgPtr; typedef AsyncCall::Pointer Callback; - HttpControlMsg(const MsgPtr &aReply, const Callback &aCallback): + HttpControlMsg(const HttpReply::Pointer &aReply, const Callback &aCallback): reply(aReply), cbSuccess(aCallback) {} public: - MsgPtr reply; ///< the 1xx message being forwarded + HttpReply::Pointer reply; ///< the 1xx message being forwarded Callback cbSuccess; ///< called after successfully writing the 1xx message // We could add an API to notify of send failures as well, but the diff --git a/src/HttpMsg.cc b/src/HttpMsg.cc index e640592fd8..f5b4e464ad 100644 --- a/src/HttpMsg.cc +++ b/src/HttpMsg.cc @@ -357,19 +357,3 @@ void HttpMsg::firstLineBuf(MemBuf& mb) packFirstLineInto(&p, true); packerClean(&p); } - -// use HTTPMSGLOCK() instead of calling this directly -HttpMsg * -HttpMsg::_lock() -{ - lock(); - return this; -} - -// use HTTPMSGUNLOCK() instead of calling this directly -void -HttpMsg::_unlock() -{ - if (unlock() == 0) - delete this; -} diff --git a/src/HttpMsg.h b/src/HttpMsg.h index 6eb09c84fd..48fb0fdf5d 100644 --- a/src/HttpMsg.h +++ b/src/HttpMsg.h @@ -39,16 +39,12 @@ #include "HttpVersion.h" #include "typedefs.h" -// common parts of HttpRequest and HttpReply - -template -class HttpMsgPointerT; - +/// common parts of HttpRequest and HttpReply class HttpMsg : public RefCountable { public: - typedef HttpMsgPointerT Pointer; + typedef RefCount Pointer; HttpMsg(http_hdr_owner_type owner); virtual ~HttpMsg(); @@ -57,9 +53,6 @@ public: void packInto(Packer * p, bool full_uri) const; - virtual HttpMsg *_lock(); // please use HTTPMSGLOCK() - virtual void _unlock(); // please use HTTPMSGUNLOCK() - ///< produce a message copy, except for a few connection-specific settings virtual HttpMsg *clone() const = 0; ///< \todo rename: not a true copy? @@ -130,58 +123,7 @@ protected: int httpMsgIsolateHeaders(const char **parse_start, int len, const char **blk_start, const char **blk_end); -#define HTTPMSGUNLOCK(a) if(a){(a)->_unlock();(a)=NULL;} -#define HTTPMSGLOCK(a) (a)->_lock() - -// TODO: replace HTTPMSGLOCK with general RefCounting and delete this class -/// safe HttpMsg pointer wrapper that locks and unlocks the message -template -class HttpMsgPointerT -{ -public: - HttpMsgPointerT(): msg(NULL) {} - explicit HttpMsgPointerT(Msg *m): msg(m) { lock(); } - virtual ~HttpMsgPointerT() { unlock(); } - - HttpMsgPointerT(const HttpMsgPointerT &p): msg(p.msg) { lock(); } - HttpMsgPointerT &operator =(const HttpMsgPointerT &p) - { if (msg != p.msg) { unlock(); msg = p.msg; lock(); } return *this; } - HttpMsgPointerT &operator =(Msg *newM) - { if (msg != newM) { unlock(); msg = newM; lock(); } return *this; } - - /// support converting a child msg pointer into a parent msg pointer - template - HttpMsgPointerT(const HttpMsgPointerT &o): msg(o.raw()) { lock(); } - - /// support assigning a child msg pointer to a parent msg pointer - template - HttpMsgPointerT &operator =(const HttpMsgPointerT &o) - { if (msg != o.raw()) { unlock(); msg = o.raw(); lock(); } return *this; } - - Msg &operator *() { return *msg; } - const Msg &operator *() const { return *msg; } - Msg *operator ->() { return msg; } - const Msg *operator ->() const { return msg; } - operator Msg *() const { return msg; } - // add more as needed - - /// public access for HttpMsgPointerT copying and assignment; avoid - Msg *raw() const { return msg; } - -protected: - void lock() { if (msg) HTTPMSGLOCK(msg); } ///< prevent msg destruction - void unlock() { HTTPMSGUNLOCK(msg); } ///< allows/causes msg destruction - -private: - Msg *msg; -}; - -/// convenience wrapper to create HttpMsgPointerT<> object based on msg type -template -inline -HttpMsgPointerT HttpMsgPointer(Msg *msg) -{ - return HttpMsgPointerT(msg); -} +#define HTTPMSGUNLOCK(a) if (a) { if ((a)->unlock() == 0) delete (a); (a)=NULL; } +#define HTTPMSGLOCK(a) (a)->lock() #endif /* SQUID_HTTPMSG_H */ diff --git a/src/HttpReply.cc b/src/HttpReply.cc index 1d91dc5415..9710eaa091 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -585,7 +585,7 @@ HttpReply::expectedBodyTooLarge(HttpRequest& request) } void -HttpReply::calcMaxBodySize(HttpRequest& request) +HttpReply::calcMaxBodySize(HttpRequest& request) const { // hack: -2 is used as "we have not calculated max body size yet" state if (bodySizeMax != -2) // already tried @@ -597,7 +597,9 @@ HttpReply::calcMaxBodySize(HttpRequest& request) return; ACLFilledChecklist ch(NULL, &request, NULL); - ch.reply = HTTPMSGLOCK(this); // XXX: this lock makes method non-const + // XXX: cont-cast becomes irrelevant when checklist is HttpReply::Pointer + ch.reply = const_cast(this); + HTTPMSGLOCK(ch.reply); for (AclSizeLimit *l = Config.ReplyBodySize; l; l = l -> next) { /* if there is no ACL list or if the ACLs listed match use this size value */ if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) { diff --git a/src/HttpReply.h b/src/HttpReply.h index a5a50f9a34..c3d7b35454 100644 --- a/src/HttpReply.h +++ b/src/HttpReply.h @@ -52,7 +52,7 @@ class HttpReply: public HttpMsg { public: - typedef HttpMsgPointerT Pointer; + typedef RefCount Pointer; MEMPROXY_CLASS(HttpReply); HttpReply(); @@ -60,13 +60,6 @@ public: virtual void reset(); - /// \par use HTTPMSGLOCK() instead of calling this directly - virtual HttpReply *_lock() { - return static_cast(HttpMsg::_lock()); - }; - - //virtual void unlock(); // only needed for debugging - /** \retval true on success \retval false and sets *error to zero when needs more data @@ -163,7 +156,7 @@ private: /** Calculates and stores maximum body size if needed. * Used by receivedBodyTooLarge() and expectedBodyTooLarge(). */ - void calcMaxBodySize(HttpRequest& request); + void calcMaxBodySize(HttpRequest& request) const; String removeStaleWarningValues(const String &value); diff --git a/src/HttpRequest.h b/src/HttpRequest.h index b348741b86..10968ac090 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -66,7 +66,7 @@ class HttpRequest: public HttpMsg { public: - typedef HttpMsgPointerT Pointer; + typedef RefCount Pointer; MEMPROXY_CLASS(HttpRequest); HttpRequest(); @@ -74,11 +74,6 @@ public: ~HttpRequest(); virtual void reset(); - // use HTTPMSGLOCK() instead of calling this directly - virtual HttpRequest *_lock() { - return static_cast(HttpMsg::_lock()); - }; - void initHTTP(const HttpRequestMethod& aMethod, AnyP::ProtocolType aProtocol, const char *aUrlpath); virtual HttpRequest *clone() const; diff --git a/src/MemObject.cc b/src/MemObject.cc index 59f8511822..7e403697a4 100644 --- a/src/MemObject.cc +++ b/src/MemObject.cc @@ -86,9 +86,9 @@ MemObject::resetUrls(char const *aUrl, char const *aLog_url) MemObject::MemObject(char const *aUrl, char const *aLog_url) { debugs(20, 3, HERE << "new MemObject " << this); - HttpReply *rep = new HttpReply; + _reply = new HttpReply; + HTTPMSGLOCK(_reply); - _reply = HTTPMSGLOCK(rep); url = xstrdup(aUrl); #if URL_CHECKSUM_DEBUG @@ -195,7 +195,8 @@ void MemObject::replaceHttpReply(HttpReply *newrep) { HTTPMSGUNLOCK(_reply); - _reply = HTTPMSGLOCK(newrep); + _reply = newrep; + HTTPMSGLOCK(_reply); } struct LowestMemReader : public unary_function { diff --git a/src/Notes.cc b/src/Notes.cc index 5a11616ee8..02c31b9c3a 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -59,8 +59,9 @@ Note::match(HttpRequest *request, HttpReply *reply) typedef Values::iterator VLI; ACLFilledChecklist ch(NULL, request, NULL); + ch.reply = reply; if (reply) - ch.reply = HTTPMSGLOCK(reply); + HTTPMSGLOCK(ch.reply); for (VLI i = values.begin(); i != values.end(); ++i ) { const int ret= ch.fastCheck((*i)->aclList); diff --git a/src/Server.cc b/src/Server.cc index 619b9f3b32..746d15d0df 100644 --- a/src/Server.cc +++ b/src/Server.cc @@ -75,7 +75,8 @@ ServerStateData::ServerStateData(FwdState *theFwdState): AsyncJob("ServerStateDa entry->lock(); - request = HTTPMSGLOCK(fwd->request); + request = fwd->request; + HTTPMSGLOCK(request); } ServerStateData::~ServerStateData() @@ -147,7 +148,8 @@ ServerStateData::setVirginReply(HttpReply *rep) debugs(11,5, HERE << this << " setting virgin reply to " << rep); assert(!theVirginReply); assert(rep); - theVirginReply = HTTPMSGLOCK(rep); + theVirginReply = rep; + HTTPMSGLOCK(theVirginReply); return theVirginReply; } @@ -165,7 +167,8 @@ ServerStateData::setFinalReply(HttpReply *rep) assert(!theFinalReply); assert(rep); - theFinalReply = HTTPMSGLOCK(rep); + theFinalReply = rep; + HTTPMSGLOCK(theFinalReply); // give entry the reply because haveParsedReplyHeaders() expects it there entry->replaceHttpReply(theFinalReply, false); // but do not write yet @@ -658,7 +661,7 @@ ServerStateData::noteAdaptationAnswer(const Adaptation::Answer &answer) switch (answer.kind) { case Adaptation::Answer::akForward: - handleAdaptedHeader(answer.message); + handleAdaptedHeader(const_cast(answer.message.getRaw())); break; case Adaptation::Answer::akBlock: diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc index b799c9ef24..f6865e7119 100644 --- a/src/acl/Asn.cc +++ b/src/acl/Asn.cc @@ -237,16 +237,15 @@ asnCacheStart(int as) { LOCAL_ARRAY(char, asres, 4096); StoreEntry *e; - HttpRequest *req; ASState *asState; asState = cbdataAlloc(ASState); asState->dataRead = 0; debugs(53, 3, "asnCacheStart: AS " << as); snprintf(asres, 4096, "whois://%s/!gAS%d", Config.as_whois_server, as); asState->as_number = as; - req = HttpRequest::CreateFromUrl(asres); - assert(NULL != req); - asState->request = HTTPMSGLOCK(req); + asState->request = HttpRequest::CreateFromUrl(asres); + assert(NULL != asState->request); + HTTPMSGLOCK(asState->request); if ((e = storeGetPublic(asres, Http::METHOD_GET)) == NULL) { e = storeCreateEntry(asres, asres, RequestFlags(), Http::METHOD_GET); diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index bddf2a5f82..a68fdccc78 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -175,7 +175,8 @@ ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_re accessList = cbdataReference(A); if (http_request != NULL) { - request = HTTPMSGLOCK(http_request); + request = http_request; + HTTPMSGLOCK(request); #if FOLLOW_X_FORWARDED_FOR if (Config.onoff.acl_uses_indirect_client) src_addr = request->indirect_client_addr; diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc index 77e5351d58..f2ce917634 100644 --- a/src/adaptation/AccessCheck.cc +++ b/src/adaptation/AccessCheck.cc @@ -122,7 +122,8 @@ Adaptation::AccessCheck::checkCandidates() /* BUG 2526: what to do when r->acl is empty?? */ // XXX: we do not have access to conn->rfc931 here. acl_checklist = new ACLFilledChecklist(r->acl, filter.request, dash_str); - acl_checklist->reply = filter.reply ? HTTPMSGLOCK(filter.reply) : NULL; + acl_checklist->reply = filter.reply; + HTTPMSGLOCK(acl_checklist->reply); acl_checklist->nonBlockingCheck(AccessCheckCallbackWrapper, this); return; } diff --git a/src/adaptation/Answer.h b/src/adaptation/Answer.h index cf7f71c983..2db80ec451 100644 --- a/src/adaptation/Answer.h +++ b/src/adaptation/Answer.h @@ -27,7 +27,7 @@ public: std::ostream &print(std::ostream &os) const; public: - HttpMsgPointerT message; ///< HTTP request or response to forward + HttpMsg::Pointer message; ///< HTTP request or response to forward String ruleId; ///< ACL (or similar rule) name that blocked forwarding bool final; ///< whether the error, if any, cannot be bypassed Kind kind; ///< the type of the answer diff --git a/src/adaptation/Iterator.cc b/src/adaptation/Iterator.cc index 71a0c404b8..11505edf2b 100644 --- a/src/adaptation/Iterator.cc +++ b/src/adaptation/Iterator.cc @@ -20,12 +20,17 @@ Adaptation::Iterator::Iterator( AsyncJob("Iterator"), Adaptation::Initiate("Iterator"), theGroup(aGroup), - theMsg(HTTPMSGLOCK(aMsg)), - theCause(aCause ? HTTPMSGLOCK(aCause) : NULL), + theMsg(aMsg), + theCause(aCause), theLauncher(0), iterations(0), adapted(false) { + if (theCause != NULL) + HTTPMSGLOCK(theCause); + + if (theMsg != NULL) + HTTPMSGLOCK(theMsg); } Adaptation::Iterator::~Iterator() @@ -85,7 +90,7 @@ Adaptation::Iterator::noteAdaptationAnswer(const Answer &answer) { switch (answer.kind) { case Answer::akForward: - handleAdaptedHeader(answer.message); + handleAdaptedHeader(const_cast(answer.message.getRaw())); break; case Answer::akBlock: @@ -115,7 +120,8 @@ Adaptation::Iterator::handleAdaptedHeader(HttpMsg *aMsg) Must(aMsg); HTTPMSGUNLOCK(theMsg); - theMsg = HTTPMSGLOCK(aMsg); + theMsg = aMsg; + HTTPMSGLOCK(theMsg); adapted = true; clearAdaptation(theLauncher); diff --git a/src/adaptation/Message.cc b/src/adaptation/Message.cc index dd5ce3867f..d3dcf62478 100644 --- a/src/adaptation/Message.cc +++ b/src/adaptation/Message.cc @@ -33,7 +33,8 @@ Adaptation::Message::set(Header *aHeader) { clear(); if (aHeader) { - header = HTTPMSGLOCK(aHeader); + header = aHeader; + HTTPMSGLOCK(header); body_pipe = header->body_pipe; } } diff --git a/src/adaptation/ServiceFilter.cc b/src/adaptation/ServiceFilter.cc index f7cd8d3989..f488eba8d7 100644 --- a/src/adaptation/ServiceFilter.cc +++ b/src/adaptation/ServiceFilter.cc @@ -3,20 +3,31 @@ #include "HttpReply.h" #include "adaptation/ServiceFilter.h" -Adaptation::ServiceFilter::ServiceFilter(Method aMethod, VectPoint aPoint, - HttpRequest *aReq, HttpReply *aRep): method(aMethod), point(aPoint), - request(HTTPMSGLOCK(aReq)), - reply(aRep ? HTTPMSGLOCK(aRep) : NULL) +Adaptation::ServiceFilter::ServiceFilter(Method aMethod, VectPoint aPoint, HttpRequest *aReq, HttpReply *aRep): + method(aMethod), + point(aPoint), + request(aReq), + reply(aRep) { + if (reply) + HTTPMSGLOCK(reply); + // a lot of code assumes that there is always a virgin request or cause assert(request); + HTTPMSGLOCK(request); } Adaptation::ServiceFilter::ServiceFilter(const ServiceFilter &f): - method(f.method), point(f.point), - request(HTTPMSGLOCK(f.request)), - reply(f.reply ? HTTPMSGLOCK(f.reply) : NULL) + method(f.method), + point(f.point), + request(f.request), + reply(f.reply) { + if (request) + HTTPMSGLOCK(request); + + if (reply) + HTTPMSGLOCK(reply); } Adaptation::ServiceFilter::~ServiceFilter() @@ -32,8 +43,11 @@ Adaptation::ServiceFilter &Adaptation::ServiceFilter::operator =(const ServiceFi point = f.point; HTTPMSGUNLOCK(request); HTTPMSGUNLOCK(reply); - request = HTTPMSGLOCK(f.request); - reply = f.reply ? HTTPMSGLOCK(f.reply) : NULL; + request = f.request; + HTTPMSGLOCK(request); + reply = f.reply; + if (reply) + HTTPMSGLOCK(reply); } return *this; } diff --git a/src/adaptation/icap/InOut.h b/src/adaptation/icap/InOut.h index e3ba4f387a..4251c93c07 100644 --- a/src/adaptation/icap/InOut.h +++ b/src/adaptation/icap/InOut.h @@ -62,7 +62,8 @@ public: void setCause(HttpRequest *r) { if (r) { HTTPMSGUNLOCK(cause); - cause = HTTPMSGLOCK(r); + cause = r; + HTTPMSGLOCK(cause); } else { assert(!cause); } @@ -70,7 +71,8 @@ public: void setHeader(Header *h) { HTTPMSGUNLOCK(header); - header = HTTPMSGLOCK(h); + header = h; + HTTPMSGLOCK(header); body_pipe = header->body_pipe; } diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc index 23facedb28..a4c1f62142 100644 --- a/src/adaptation/icap/Launcher.cc +++ b/src/adaptation/icap/Launcher.cc @@ -136,7 +136,8 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info ACLFilledChecklist *cl = new ACLFilledChecklist(TheConfig.repeat, info.icapRequest, dash_str); - cl->reply = HTTPMSGLOCK(info.icapReply); + cl->reply = info.icapReply; + HTTPMSGLOCK(cl->reply); bool result = cl->fastCheck() == ACCESS_ALLOWED; delete cl; @@ -147,17 +148,27 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info Adaptation::Icap::XactAbortInfo::XactAbortInfo(HttpRequest *anIcapRequest, HttpReply *anIcapReply, bool beRetriable, bool beRepeatable): - icapRequest(anIcapRequest ? HTTPMSGLOCK(anIcapRequest) : NULL), - icapReply(anIcapReply ? HTTPMSGLOCK(anIcapReply) : NULL), - isRetriable(beRetriable), isRepeatable(beRepeatable) + icapRequest(anIcapRequest), + icapReply(anIcapReply), + isRetriable(beRetriable), + isRepeatable(beRepeatable) { + if (icapRequest) + HTTPMSGLOCK(icapRequest); + if (icapReply) + HTTPMSGLOCK(icapReply); } Adaptation::Icap::XactAbortInfo::XactAbortInfo(const Adaptation::Icap::XactAbortInfo &i): - icapRequest(i.icapRequest ? HTTPMSGLOCK(i.icapRequest) : NULL), - icapReply(i.icapReply ? HTTPMSGLOCK(i.icapReply) : NULL), - isRetriable(i.isRetriable), isRepeatable(i.isRepeatable) + icapRequest(i.icapRequest), + icapReply(i.icapReply), + isRetriable(i.isRetriable), + isRepeatable(i.isRepeatable) { + if (icapRequest) + HTTPMSGLOCK(icapRequest); + if (icapReply) + HTTPMSGLOCK(icapReply); } Adaptation::Icap::XactAbortInfo::~XactAbortInfo() diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index a123754d12..72086a9b45 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -771,7 +771,7 @@ void Adaptation::Icap::ModXact::parseIcapHead() { Must(state.sending == State::sendingUndecided); - if (!parseHead(icapReply)) + if (!parseHead(icapReply.getRaw())) return; if (httpHeaderHasConnDir(&icapReply->header, "close")) { @@ -946,8 +946,7 @@ void Adaptation::Icap::ModXact::prepEchoing() { HttpMsg::Pointer newHead; if (dynamic_cast(oldHead)) { - HttpRequest::Pointer newR(new HttpRequest); - newHead = newR; + newHead = new HttpRequest; } else if (dynamic_cast(oldHead)) { newHead = new HttpReply; } @@ -955,7 +954,7 @@ void Adaptation::Icap::ModXact::prepEchoing() newHead->inheritProperties(oldHead); - adapted.setHeader(newHead); + adapted.setHeader(newHead.getRaw()); } // parse the buffer back @@ -1269,10 +1268,12 @@ void Adaptation::Icap::ModXact::finalizeLogInfo() al.cache.caddr = request_->client_addr; - al.request = HTTPMSGLOCK(request_); - if (reply_) - al.reply = HTTPMSGLOCK(reply_); - else + al.request = request_; + HTTPMSGLOCK(al.request); + if (reply_) { + al.reply = reply_; + HTTPMSGLOCK(al.reply); + } else al.reply = NULL; if (h->rfc931.size()) @@ -1514,13 +1515,13 @@ void Adaptation::Icap::ModXact::encapsulateHead(MemBuf &icapBuf, const char *sec if (const HttpRequest* old_request = dynamic_cast(head)) { HttpRequest::Pointer new_request(new HttpRequest); Must(old_request->canonical); - urlParse(old_request->method, old_request->canonical, new_request); + urlParse(old_request->method, old_request->canonical, new_request.getRaw()); new_request->http_ver = old_request->http_ver; - headClone = new_request; + headClone = new_request.getRaw(); } else if (const HttpReply *old_reply = dynamic_cast(head)) { HttpReply::Pointer new_reply(new HttpReply); new_reply->sline = old_reply->sline; - headClone = new_reply; + headClone = new_reply.getRaw(); } Must(headClone != NULL); headClone->inheritProperties(head); @@ -1537,7 +1538,7 @@ void Adaptation::Icap::ModXact::encapsulateHead(MemBuf &icapBuf, const char *sec headClone->header.removeHopByHopEntries(); // pack polished HTTP header - packHead(httpBuf, headClone); + packHead(httpBuf, headClone.getRaw()); // headClone unlocks and, hence, deletes the message we packed } diff --git a/src/adaptation/icap/OptXact.cc b/src/adaptation/icap/OptXact.cc index d4df5d3003..e4da1d18d2 100644 --- a/src/adaptation/icap/OptXact.cc +++ b/src/adaptation/icap/OptXact.cc @@ -79,7 +79,7 @@ void Adaptation::Icap::OptXact::handleCommRead(size_t) debugs(93, 7, HERE << "readAll=" << readAll); icap_tio_finish = current_time; setOutcome(xoOpt); - sendAnswer(Answer::Forward(icapReply)); + sendAnswer(Answer::Forward(icapReply.getRaw())); Must(done()); // there should be nothing else to do return; } @@ -96,7 +96,7 @@ bool Adaptation::Icap::OptXact::parseResponse() HttpReply::Pointer r(new HttpReply); r->protoPrefix = "ICAP/"; // TODO: make an IcapReply class? - if (!parseHttpMsg(r)) // throws on errors + if (!parseHttpMsg(r.getRaw())) // throws on errors return false; if (httpHeaderHasConnDir(&r->header, "close")) @@ -116,7 +116,7 @@ void Adaptation::Icap::OptXact::finalizeLogInfo() // al.cache.caddr = 0; al.icap.reqMethod = Adaptation::methodOptions; - if (icapReply && al.icap.bytesRead > icapReply->hdr_sz) + if (icapReply != NULL && al.icap.bytesRead > icapReply->hdr_sz) al.icap.bodyBytesRead = al.icap.bytesRead - icapReply->hdr_sz; Adaptation::Icap::Xaction::finalizeLogInfo(); diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc index 30110d435b..6bc1e3032b 100644 --- a/src/adaptation/icap/ServiceRep.cc +++ b/src/adaptation/icap/ServiceRep.cc @@ -530,13 +530,13 @@ void Adaptation::Icap::ServiceRep::noteAdaptationAnswer(const Answer &answer) } Must(answer.kind == Answer::akForward); // no akBlock for OPTIONS requests - HttpMsg *msg = answer.message; + const HttpMsg *msg = answer.message.getRaw(); Must(msg); debugs(93,5, HERE << "is interpreting new options " << status()); Adaptation::Icap::Options *newOptions = NULL; - if (HttpReply *r = dynamic_cast(msg)) { + if (const HttpReply *r = dynamic_cast(msg)) { newOptions = new Adaptation::Icap::Options; newOptions->configure(r); } else { diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc index 34238f260d..996be69db8 100644 --- a/src/adaptation/icap/Xaction.cc +++ b/src/adaptation/icap/Xaction.cc @@ -50,7 +50,8 @@ Adaptation::Icap::Xaction::Xaction(const char *aTypeName, Adaptation::Icap::Serv { debugs(93,3, typeName << " constructed, this=" << this << " [icapx" << id << ']'); // we should not call virtual status() here - icapRequest = HTTPMSGLOCK(new HttpRequest); + icapRequest = new HttpRequest; + HTTPMSGLOCK(icapRequest); icap_tr_start = current_time; } @@ -536,7 +537,7 @@ void Adaptation::Icap::Xaction::swanSong() void Adaptation::Icap::Xaction::tellQueryAborted() { if (theInitiator.set()) { - Adaptation::Icap::XactAbortInfo abortInfo(icapRequest, icapReply, + Adaptation::Icap::XactAbortInfo abortInfo(icapRequest, icapReply.getRaw(), retriable(), repeatable()); Launcher *launcher = dynamic_cast(theInitiator.get()); // launcher may be nil if initiator is invalid @@ -571,9 +572,11 @@ void Adaptation::Icap::Xaction::finalizeLogInfo() al.icap.ioTime = tvSubMsec(icap_tio_start, icap_tio_finish); al.icap.trTime = tvSubMsec(icap_tr_start, current_time); - al.icap.request = HTTPMSGLOCK(icapRequest); - if (icapReply) { - al.icap.reply = HTTPMSGLOCK(icapReply); + al.icap.request = icapRequest; + HTTPMSGLOCK(al.icap.request); + if (icapReply != NULL) { + al.icap.reply = icapReply.getRaw(); + HTTPMSGLOCK(al.icap.reply); al.icap.resStatus = icapReply->sline.status; } } diff --git a/src/client_side.cc b/src/client_side.cc index f60df29f13..af2c1a6ad5 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -381,8 +381,8 @@ ClientSocketContextNew(const Comm::ConnectionPointer &client, ClientHttpRequest void ClientSocketContext::writeControlMsg(HttpControlMsg &msg) { - HttpReply *rep = msg.reply; - Must(rep); + const HttpReply::Pointer rep(msg.reply); + Must(rep != NULL); // apply selected clientReplyContext::buildReplyHeader() mods // it is not clear what headers are required for control messages @@ -707,12 +707,16 @@ ClientHttpRequest::logRequest() ACLFilledChecklist *checklist = clientAclChecklistCreate(Config.accessList.log, this); - if (al->reply) - checklist->reply = HTTPMSGLOCK(al->reply); + if (al->reply) { + checklist->reply = al->reply; + HTTPMSGLOCK(checklist->reply); + } if (!Config.accessList.log || checklist->fastCheck() == ACCESS_ALLOWED) { - if (request) - al->adapted_request = HTTPMSGLOCK(request); + if (request) { + al->adapted_request = request; + HTTPMSGLOCK(al->adapted_request); + } accessLogLog(al, checklist); if (request) updateCounters(); @@ -1475,7 +1479,8 @@ clientSocketRecipient(clientStreamNode * node, ClientHttpRequest * http, context->sendBody(rep, receivedData); else { assert(rep); - http->al->reply = HTTPMSGLOCK(rep); + http->al->reply = rep; + HTTPMSGLOCK(http->al->reply); context->sendStartOfMessage(rep, receivedData); } @@ -2517,8 +2522,10 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context) repContext->setReplyToStoreEntry(sslServerBump->entry); // save the original request for logging purposes - if (!context->http->al->request) - context->http->al->request = HTTPMSGLOCK(http->request); + if (!context->http->al->request) { + context->http->al->request = http->request; + HTTPMSGLOCK(context->http->al->request); + } // Get error details from the fake certificate-peeking request. http->request->detailError(sslServerBump->request->errType, sslServerBump->request->errDetail); @@ -2561,8 +2568,10 @@ bool ConnStateData::serveDelayedError(ClientSocketContext *context) sslServerBump->serverCert.get(), NULL); err->detail = errDetail; // Save the original request for logging purposes. - if (!context->http->al->request) - context->http->al->request = HTTPMSGLOCK(request); + if (!context->http->al->request) { + context->http->al->request = request; + HTTPMSGLOCK(context->http->al->request); + } repContext->setReplyToError(request->method, err); assert(context->http->out.offset == 0); context->pullData(); @@ -2618,7 +2627,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, method)) == NULL) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Invalid URL: " << http->uri); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); @@ -2637,7 +2646,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Unsupported HTTP version discovered. :\n" << HttpParserHdrBuf(hp)); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); @@ -2655,7 +2664,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c if (http_ver.major >= 1 && !request->parseHeader(HttpParserHdrBuf(hp), HttpParserHdrSz(hp))) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Failed to parse request headers:\n" << HttpParserHdrBuf(hp)); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); // setLogUri should called before repContext->setReplyToError setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); @@ -2707,7 +2716,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c } request->flags.internal = http->flags.internal; - setLogUri (http, urlCanonicalClean(request)); + setLogUri (http, urlCanonicalClean(request.getRaw())); request->client_addr = conn->clientConnection->remote; // XXX: remove reuest->client_addr member. #if FOLLOW_X_FORWARDED_FOR // indirect client gets stored here because it is an HTTP header result (from X-Forwarded-For:) @@ -2732,26 +2741,26 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c mustReplyToOptions = (method == Http::METHOD_OPTIONS) && (request->header.getInt64(HDR_MAX_FORWARDS) == 0); - if (!urlCheckRequest(request) || mustReplyToOptions || unsupportedTe) { + if (!urlCheckRequest(request.getRaw()) || mustReplyToOptions || unsupportedTe) { clientStreamNode *node = context->getClientReplyContext(); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_UNSUP_REQ, HTTP_NOT_IMPLEMENTED, request->method, NULL, - conn->clientConnection->remote, request, NULL, NULL); + conn->clientConnection->remote, request.getRaw(), NULL, NULL); assert(context->http->out.offset == 0); context->pullData(); goto finish; } - if (!chunked && !clientIsContentLengthValid(request)) { + if (!chunked && !clientIsContentLengthValid(request.getRaw())) { clientStreamNode *node = context->getClientReplyContext(); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); repContext->setReplyToError(ERR_INVALID_REQ, HTTP_LENGTH_REQUIRED, request->method, NULL, - conn->clientConnection->remote, request, NULL, NULL); + conn->clientConnection->remote, request.getRaw(), NULL, NULL); assert(context->http->out.offset == 0); context->pullData(); goto finish; @@ -2764,16 +2773,17 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c clientStreamNode *node = context->getClientReplyContext(); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); repContext->setReplyToError(ERR_INVALID_REQ, HTTP_EXPECTATION_FAILED, request->method, http->uri, - conn->clientConnection->remote, request, NULL, NULL); + conn->clientConnection->remote, request.getRaw(), NULL, NULL); assert(context->http->out.offset == 0); context->pullData(); goto finish; } } - http->request = HTTPMSGLOCK(request); + http->request = request.getRaw(); + HTTPMSGLOCK(http->request); clientSetKeepaliveFlag(http); // Let tunneling code be fully responsible for CONNECT requests @@ -2803,7 +2813,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c clientStreamNode *node = context->getClientReplyContext(); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); - conn->quitAfterError(request); + conn->quitAfterError(request.getRaw()); repContext->setReplyToError(ERR_TOO_BIG, HTTP_REQUEST_ENTITY_TOO_LARGE, Http::METHOD_NONE, NULL, conn->clientConnection->remote, http->request, NULL, NULL); @@ -2839,7 +2849,7 @@ finish: * be freed and the above connNoteUseOfBuffer() would hit an * assertion, not to mention that we were accessing freed memory. */ - if (request && request->flags.resetTcp && Comm::IsConnOpen(conn->clientConnection)) { + if (request != NULL && request->flags.resetTcp && Comm::IsConnOpen(conn->clientConnection)) { debugs(33, 3, HERE << "Sending TCP RST on " << conn->clientConnection); conn->flags.readMore = false; comm_reset_close(conn->clientConnection); @@ -3751,7 +3761,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer if (X509 *mimicCert = sslServerBump->serverCert.get()) certProperties.mimicCert.resetAndLock(mimicCert); - ACLFilledChecklist checklist(NULL, sslServerBump->request, + ACLFilledChecklist checklist(NULL, sslServerBump->request.getRaw(), clientConnection != NULL ? clientConnection->rfc931 : dash_str); checklist.sslErrors = cbdataReference(sslServerBump->sslErrors); @@ -3936,7 +3946,7 @@ ConnStateData::switchToHttps(HttpRequest *request, Ssl::BumpMode bumpServerMode) sslServerBump = new Ssl::ServerBump(request); // will call httpsPeeked() with certificate and connection, eventually - FwdState::fwdStart(clientConnection, sslServerBump->entry, sslServerBump->request); + FwdState::fwdStart(clientConnection, sslServerBump->entry, sslServerBump->request.getRaw()); return; } diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index f34b60dd6c..412011c3e6 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -1542,9 +1542,8 @@ clientReplyContext::cloneReply() { assert(reply == NULL); - HttpReply *rep = http->storeEntry()->getReply()->clone(); - - reply = HTTPMSGLOCK(rep); + reply = http->storeEntry()->getReply()->clone(); + HTTPMSGLOCK(reply); if (reply->sline.protocol == AnyP::PROTO_HTTP) { /* RFC 2616 requires us to advertise our 1.1 version (but only on real HTTP traffic) */ @@ -1960,7 +1959,8 @@ clientReplyContext::processReplyAccess () /** Process http_reply_access lists */ ACLFilledChecklist *replyChecklist = clientAclChecklistCreate(Config.accessList.reply, http); - replyChecklist->reply = HTTPMSGLOCK(reply); + replyChecklist->reply = reply; + HTTPMSGLOCK(replyChecklist->reply); replyChecklist->nonBlockingCheck(ProcessReplyAccessResult, this); } @@ -2178,8 +2178,10 @@ clientReplyContext::createStoreEntry(const HttpRequestMethod& m, RequestFlags re * so make a fake one. */ - if (http->request == NULL) - http->request = HTTPMSGLOCK(new HttpRequest(m, AnyP::PROTO_NONE, null_string)); + if (http->request == NULL) { + http->request = new HttpRequest(m, AnyP::PROTO_NONE, null_string); + HTTPMSGLOCK(http->request); + } StoreEntry *e = storeCreateEntry(storeId(), http->log_uri, reqFlags, m); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 8d95b91996..d77846aa44 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -422,7 +422,8 @@ clientBeginRequest(const HttpRequestMethod& method, char const *url, CSCB * stre HttpVersion http_ver(1,1); request->http_ver = http_ver; - http->request = HTTPMSGLOCK(request); + http->request = request; + HTTPMSGLOCK(http->request); /* optional - skip the access check ? */ http->calloutContext = new ClientRequestContext(http); @@ -1343,7 +1344,8 @@ ClientRequestContext::clientRedirectDone(const HelperReply &reply) safe_free(http->uri); http->uri = xstrdup(urlCanonical(new_request)); HTTPMSGUNLOCK(old_request); - http->request = HTTPMSGLOCK(new_request); + http->request = new_request; + HTTPMSGLOCK(http->request); } else { debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid request: " << old_request->method << " " << urlNote->firstValue() << " " << old_request->http_ver); @@ -1703,8 +1705,10 @@ ClientHttpRequest::doCallouts() assert(calloutContext); /*Save the original request for logging purposes*/ - if (!calloutContext->http->al->request) - calloutContext->http->al->request = HTTPMSGLOCK(request); + if (!calloutContext->http->al->request) { + calloutContext->http->al->request = request; + HTTPMSGLOCK(calloutContext->http->al->request); + } if (!calloutContext->error) { // CVE-2009-0801: verify the Host: header is consistent with other known details. @@ -1892,7 +1896,7 @@ ClientHttpRequest::noteAdaptationAnswer(const Adaptation::Answer &answer) switch (answer.kind) { case Adaptation::Answer::akForward: - handleAdaptedHeader(answer.message); + handleAdaptedHeader(const_cast(answer.message.getRaw())); break; case Adaptation::Answer::akBlock: @@ -1915,7 +1919,8 @@ ClientHttpRequest::handleAdaptedHeader(HttpMsg *msg) * Replace the old request with the new request. */ HTTPMSGUNLOCK(request); - request = HTTPMSGLOCK(new_req); + request = new_req; + HTTPMSGLOCK(request); /* * Store the new URI for logging */ diff --git a/src/errorpage.cc b/src/errorpage.cc index 635fddef90..a8d4f452c5 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -432,7 +432,7 @@ bool strHdrAcptLangGetItem(const String &hdr, char *lang, int langLen, size_t &p } bool -TemplateFile::loadFor(HttpRequest *request) +TemplateFile::loadFor(const HttpRequest *request) { String hdr; @@ -600,7 +600,8 @@ ErrorState::ErrorState(err_type t, http_status status, HttpRequest * req) : httpStatus = ErrorDynamicPages.items[page_id - ERR_MAX]->page_redirect; if (req != NULL) { - request = HTTPMSGLOCK(req); + request = req; + HTTPMSGLOCK(request); src_addr = req->client_addr; } } diff --git a/src/errorpage.h b/src/errorpage.h index 42c1964d68..dd7bdc1ffe 100644 --- a/src/errorpage.h +++ b/src/errorpage.h @@ -278,7 +278,7 @@ public: * template selected (eg because of a "Accept-Language: *"), or not available * template found this function return false. */ - bool loadFor(HttpRequest *request); + bool loadFor(const HttpRequest *request); /** * Load the file given by "path". It uses the "parse()" method. diff --git a/src/forward.cc b/src/forward.cc index 5e0d1c470e..d06577df8a 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -124,7 +124,8 @@ FwdState::FwdState(const Comm::ConnectionPointer &client, StoreEntry * e, HttpRe debugs(17, 2, HERE << "Forwarding client request " << client << ", url=" << e->url() ); entry = e; clientConn = client; - request = HTTPMSGLOCK(r); + request = r; + HTTPMSGLOCK(request); pconnRace = raceImpossible; start_t = squid_curtime; serverDestinations.reserve(Config.forward_max_tries); @@ -322,7 +323,8 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht * This seems like an odd place to bind mem_obj and request. * Might want to assert that request is NULL at this point */ - entry->mem_obj->request = HTTPMSGLOCK(request); + entry->mem_obj->request = request; + HTTPMSGLOCK(entry->mem_obj->request); #if URL_CHECKSUM_DEBUG entry->mem_obj->checkUrlChecksum(); @@ -402,8 +404,10 @@ FwdState::fail(ErrorState * errorState) delete err; err = errorState; - if (!errorState->request) - errorState->request = HTTPMSGLOCK(request); + if (!errorState->request) { + errorState->request = request; + HTTPMSGLOCK(errorState->request); + } if (err->type != ERR_ZERO_SIZE_OBJECT) return; diff --git a/src/http.cc b/src/http.cc index d8a6050b3c..0cf313a58b 100644 --- a/src/http.cc +++ b/src/http.cc @@ -772,7 +772,7 @@ HttpStateData::processReplyHeader() void HttpStateData::handle1xx(HttpReply *reply) { - HttpMsgPointerT msg(reply); // will destroy reply if unused + HttpReply::Pointer msg(reply); // will destroy reply if unused // one 1xx at a time: we must not be called while waiting for previous 1xx Must(!flags.handling1xx); @@ -788,7 +788,8 @@ HttpStateData::handle1xx(HttpReply *reply) // check whether the 1xx response forwarding is allowed by squid.conf if (Config.accessList.reply) { ACLFilledChecklist ch(Config.accessList.reply, originalRequest(), NULL); - ch.reply = HTTPMSGLOCK(reply); + ch.reply = reply; + HTTPMSGLOCK(ch.reply); if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups? debugs(11, 3, HERE << "ignoring denied 1xx"); proceedAfter1xx(); diff --git a/src/icp_v2.cc b/src/icp_v2.cc index ee5b60dd94..d821ba2776 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -132,10 +132,12 @@ _icp_common_t::getOpCode() const ICPState::ICPState(icp_common_t &aHeader, HttpRequest *aRequest): header(aHeader), - request(HTTPMSGLOCK(aRequest)), + request(aRequest), fd(-1), url(NULL) -{} +{ + HTTPMSGLOCK(request); +} ICPState::~ICPState() { diff --git a/src/mime.cc b/src/mime.cc index b20d50b33c..7531e6834d 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -466,7 +466,8 @@ MimeIcon::created (StoreEntry *newEntry) if (NULL == r) fatal("mimeLoadIcon: cannot parse internal URL"); - e->mem_obj->request = HTTPMSGLOCK(r); + e->mem_obj->request = r; + HTTPMSGLOCK(e->mem_obj->request); HttpReply *reply = new HttpReply; diff --git a/src/neighbors.cc b/src/neighbors.cc index a958695431..da99ce3df6 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -1381,13 +1381,15 @@ peerCountMcastPeersStart(void *data) fake = storeCreateEntry(url, url, RequestFlags(), Http::METHOD_GET); HttpRequest *req = HttpRequest::CreateFromUrl(url); psstate = new ps_state; - psstate->request = HTTPMSGLOCK(req); + psstate->request = req; + HTTPMSGLOCK(psstate->request); psstate->entry = fake; psstate->callback = NULL; psstate->callback_data = cbdataReference(p); psstate->ping.start = current_time; mem = fake->mem_obj; - mem->request = HTTPMSGLOCK(psstate->request); + mem->request = psstate->request; + HTTPMSGLOCK(mem->request); mem->start_ping = current_time; mem->ping_reply_callback = peerCountHandleIcpReply; mem->ircb_data = psstate; diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 6e58defbe7..1e723329d2 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -355,7 +355,8 @@ peerDigestRequest(PeerDigest * pd) fetch = cbdataAlloc(DigestFetchState); - fetch->request = HTTPMSGLOCK(req); + fetch->request = req; + HTTPMSGLOCK(fetch->request); fetch->pd = cbdataReference(pd); @@ -555,8 +556,10 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size) /* our old entry is fine */ assert(fetch->old_entry); - if (!fetch->old_entry->mem_obj->request) - fetch->old_entry->mem_obj->request = HTTPMSGLOCK(fetch->entry->mem_obj->request); + if (!fetch->old_entry->mem_obj->request) { + fetch->old_entry->mem_obj->request = fetch->entry->mem_obj->request; + HTTPMSGLOCK(fetch->old_entry->mem_obj->request); + } assert(fetch->old_entry->mem_obj->request); diff --git a/src/peer_select.cc b/src/peer_select.cc index add3165ac9..bfd3a1f541 100644 --- a/src/peer_select.cc +++ b/src/peer_select.cc @@ -157,7 +157,8 @@ peerSelect(Comm::ConnectionList * paths, psstate = new ps_state; - psstate->request = HTTPMSGLOCK(request); + psstate->request = request; + HTTPMSGLOCK(psstate->request); psstate->entry = entry; psstate->paths = paths; diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc index 95a06f2d4d..779520858f 100644 --- a/src/ssl/ErrorDetail.cc +++ b/src/ssl/ErrorDetail.cc @@ -390,7 +390,7 @@ void Ssl::ErrorDetail::buildDetail() const char const *t; int code_len = 0; - if (ErrorDetailsManager::GetInstance().getErrorDetail(error_no, request.raw(), detailEntry)) + if (ErrorDetailsManager::GetInstance().getErrorDetail(error_no, request, detailEntry)) s = detailEntry.detail.termedBuf(); if (!s) diff --git a/src/ssl/ErrorDetailManager.cc b/src/ssl/ErrorDetailManager.cc index 0e4ae9560b..4f0624186d 100644 --- a/src/ssl/ErrorDetailManager.cc +++ b/src/ssl/ErrorDetailManager.cc @@ -115,11 +115,11 @@ void Ssl::ErrorDetailsManager::cacheDetails(ErrorDetailsList::Pointer &errorDeta } bool -Ssl::ErrorDetailsManager::getErrorDetail(Ssl::ssl_error_t value, HttpRequest *request, ErrorDetailEntry &entry) +Ssl::ErrorDetailsManager::getErrorDetail(Ssl::ssl_error_t value, const HttpRequest::Pointer &request, ErrorDetailEntry &entry) { #if USE_ERR_LOCALES String hdr; - if (request && request->header.getList(HDR_ACCEPT_LANGUAGE, &hdr)) { + if (request != NULL && request->header.getList(HDR_ACCEPT_LANGUAGE, &hdr)) { ErrorDetailsList::Pointer errDetails = NULL; //Try to retrieve from cache size_t pos = 0; @@ -132,7 +132,7 @@ Ssl::ErrorDetailsManager::getErrorDetail(Ssl::ssl_error_t value, HttpRequest *re debugs(83, 8, HERE << "Creating new ErrDetailList to read from disk"); errDetails = new ErrorDetailsList(); ErrorDetailFile detailTmpl(errDetails); - if (detailTmpl.loadFor(request)) { + if (detailTmpl.loadFor(request.getRaw())) { if (detailTmpl.language()) { debugs(83, 8, HERE << "Found details on disk for language " << detailTmpl.language()); errDetails->errLanguage = detailTmpl.language(); diff --git a/src/ssl/ErrorDetailManager.h b/src/ssl/ErrorDetailManager.h index a2809c884a..492e74032c 100644 --- a/src/ssl/ErrorDetailManager.h +++ b/src/ssl/ErrorDetailManager.h @@ -69,7 +69,7 @@ public: * \param entry where to store error details * \return true on success, false otherwise */ - bool getErrorDetail(Ssl::ssl_error_t value, HttpRequest *request, ErrorDetailEntry &entry); + bool getErrorDetail(Ssl::ssl_error_t value, const HttpRequest::Pointer &request, ErrorDetailEntry &entry); const char *getDefaultErrorDescr(Ssl::ssl_error_t value); ///< the default error description for a given error const char *getDefaultErrorDetail(Ssl::ssl_error_t value); ///< the default error details for a given error diff --git a/src/ssl/ServerBump.cc b/src/ssl/ServerBump.cc index 10d7182858..38e7234092 100644 --- a/src/ssl/ServerBump.cc +++ b/src/ssl/ServerBump.cc @@ -19,7 +19,7 @@ Ssl::ServerBump::ServerBump(HttpRequest *fakeRequest, StoreEntry *e): sslErrors(NULL) { debugs(33, 4, HERE << "will peek at " << request->GetHost() << ':' << request->port); - const char *uri = urlCanonical(request); + const char *uri = urlCanonical(request.getRaw()); if (e) { entry = e; entry->lock(); diff --git a/src/store_digest.cc b/src/store_digest.cc index 74e902e9e0..6420d82df7 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -398,7 +398,8 @@ storeDigestRewriteStart(void *datanotused) sd_state.rewrite_lock = e; debugs(71, 3, "storeDigestRewrite: url: " << url << " key: " << e->getMD5Text()); HttpRequest *req = HttpRequest::CreateFromUrl(url); - e->mem_obj->request = HTTPMSGLOCK(req); + e->mem_obj->request = req; + HTTPMSGLOCK(e->mem_obj->request); /* wait for rebuild (if any) to finish */ if (sd_state.rebuild_lock) { diff --git a/src/tests/stub_errorpage.cc b/src/tests/stub_errorpage.cc index 90eb844d2d..4a72d128e2 100644 --- a/src/tests/stub_errorpage.cc +++ b/src/tests/stub_errorpage.cc @@ -9,4 +9,4 @@ void errorAppendEntry(StoreEntry * entry, ErrorState * err) STUB bool strHdrAcptLangGetItem(const String &hdr, char *lang, int langLen, size_t &pos) STUB_RETVAL(false) bool TemplateFile::loadDefault() STUB_RETVAL(false) TemplateFile::TemplateFile(char const*, err_type) STUB -bool TemplateFile::loadFor(HttpRequest*) STUB_RETVAL(false) +bool TemplateFile::loadFor(const HttpRequest *) STUB_RETVAL(false) diff --git a/src/tunnel.cc b/src/tunnel.cc index 2697a434b8..f3bfd8e217 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -686,7 +686,8 @@ tunnelStart(ClientHttpRequest * http, int64_t * size_ptr, int *status_ptr) tunnelState->server.setDelayId(DelayId::DelayClient(http)); #endif tunnelState->url = xstrdup(url); - tunnelState->request = HTTPMSGLOCK(request); + tunnelState->request = request; + HTTPMSGLOCK(tunnelState->request); tunnelState->server.size_ptr = size_ptr; tunnelState->status_ptr = status_ptr; tunnelState->client.conn = http->getConn()->clientConnection; diff --git a/src/urn.cc b/src/urn.cc index cf4ce339f3..bbd61fd712 100644 --- a/src/urn.cc +++ b/src/urn.cc @@ -239,7 +239,8 @@ UrnState::start(HttpRequest * r, StoreEntry * e) { debugs(52, 3, "urnStart: '" << e->url() << "'" ); entry = e; - request = HTTPMSGLOCK(r); + request = r; + HTTPMSGLOCK(request); entry->lock(); setUriResFromRequest(r); -- 2.39.2