From: Francesco Chemolli <kinkie@squid-cache.org>
Date: Thu, 13 Sep 2012 16:20:41 +0000 (+0200)
Subject: Finished implementing getters/setters for RequestFlags
X-Git-Tag: sourceformat-review-1~6^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=29615259d0370de4fa1bec8bdc8604d7514feec4;p=thirdparty%2Fsquid.git

Finished implementing getters/setters for RequestFlags
---

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 95067ca56c..bfe45e759f 100644
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -611,7 +611,7 @@ HttpRequest::cacheable() const
 bool
 HttpRequest::conditional() const
 {
-    return flags.ims ||
+    return flags.hasIMS() ||
            header.has(HDR_IF_MATCH) ||
            header.has(HDR_IF_NONE_MATCH);
 }
diff --git a/src/RequestFlags.h b/src/RequestFlags.h
index 382401c694..43685e5e8a 100644
--- a/src/RequestFlags.h
+++ b/src/RequestFlags.h
@@ -35,8 +35,8 @@
 class RequestFlags {
 public:
     RequestFlags():
-        nocache(0), ims(0), auth(0), cachable(0),
-        hierarchical(0), loopdetect(false), proxy_keepalive(false), proxying_(false),
+        nocache(false), ims(false), auth_(false), cachable(false),
+        hierarchical_(false), loopdetect(false), proxy_keepalive(false), proxying_(false),
         refresh_(false), redirected(false), need_validation(false),
         fail_on_validation_err(false), stale_if_hit(false), nocache_hack(false), accelerated_(false),
         ignore_cc(false), intercepted_(false), hostVerified_(false), spoof_client_ip(false),
@@ -48,12 +48,6 @@ public:
         isRanged_(false)
     {}
 
-    unsigned int nocache :1; ///< whether the response to this request may be READ from cache
-    unsigned int ims :1;
-    unsigned int auth :1;
-    unsigned int cachable :1; ///< whether the response to thie request may be stored in the cache
-    unsigned int hierarchical :1;
-
     // When adding new flags, please update cloneAdaptationImmune() as needed.
     bool resetTCP() const;
     void setResetTCP();
@@ -170,7 +164,30 @@ public:
 
     bool loopDetect() const { return loopdetect; }
     void setLoopDetect() { loopdetect = 1; }
+
+    bool hierarchical() const { return hierarchical_; }
+    void setHierarchical() { hierarchical_=true; }
+    void clearHierarchical() { hierarchical_=true; }
+
+    bool isCachable() const { return cachable; }
+    void setCachable(bool newValue=true) { cachable = newValue; }
+    void setNotCachable() { cachable = false; }
+
+    bool hasAuth() const { return auth_; }
+    void markAuth() { auth_=true; }
+
+    bool hasIMS() const { return ims; }
+    void setIMS() { ims=true; }
+    void clearIMS() { ims=false; }
+
+    bool noCache() const { return nocache; }
+    void setNocache() { nocache=true;}
 private:
+    bool nocache :1; ///< whether the response to this request may be READ from cache
+    bool ims :1;
+    bool auth_ :1;
+    bool cachable :1; ///< whether the response to thie request may be stored in the cache
+    bool hierarchical_ :1;
     bool loopdetect :1;
     bool proxy_keepalive :1;
     bool proxying_ :1; /* this should be killed, also in httpstateflags */
diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index ae12a1fde1..7b40cebb1a 100644
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -398,7 +398,7 @@ clientReplyContext::handleIMSReply(StoreIOBuffer result)
 
         // if client sent IMS
 
-        if (http->request->flags.ims && !old_entry->modifiedSince(http->request)) {
+        if (http->request->flags.hasIMS() && !old_entry->modifiedSince(http->request)) {
             // forward the 304 from origin
             debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client");
             sendClientUpstreamResponse();
@@ -566,7 +566,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result)
              */
             http->logType = LOG_TCP_MISS;
             processMiss();
-        } else if (r->flags.nocache) {
+        } else if (r->flags.noCache()) {
             /*
              * This did not match a refresh pattern that overrides no-cache
              * we should honour the client no-cache header.
@@ -729,7 +729,7 @@ clientReplyContext::processConditional(StoreIOBuffer &result)
     if (r.header.has(HDR_IF_NONE_MATCH)) {
         if (!e->hasIfNoneMatchEtag(r)) {
             // RFC 2616: ignore IMS if If-None-Match did not match
-            r.flags.ims = 0;
+            r.flags.clearIMS();
             r.ims = -1;
             r.imslen = 0;
             r.header.delById(HDR_IF_MODIFIED_SINCE);
@@ -738,7 +738,7 @@ clientReplyContext::processConditional(StoreIOBuffer &result)
             return;
         }
 
-        if (!r.flags.ims) {
+        if (!r.flags.hasIMS()) {
             // RFC 2616: if If-None-Match matched and there is no IMS,
             // reply with 304 Not Modified or 412 Precondition Failed
             sendNotModifiedOrPreconditionFailedError();
@@ -749,7 +749,7 @@ clientReplyContext::processConditional(StoreIOBuffer &result)
         matchedIfNoneMatch = true;
     }
 
-    if (r.flags.ims) {
+    if (r.flags.hasIMS()) {
         // handle If-Modified-Since requests from the client
         if (e->modifiedSince(&r)) {
             http->logType = LOG_TCP_IMS_HIT;
@@ -1555,7 +1555,7 @@ clientReplyContext::identifyStoreObject()
 {
     HttpRequest *r = http->request;
 
-    if (r->flags.cachable || r->flags.isInternal()) {
+    if (r->flags.isCachable() || r->flags.isInternal()) {
         lookingforstore = 5;
         StoreEntry::getPublicByRequest (this, r);
     } else {
@@ -1586,7 +1586,7 @@ clientReplyContext::identifyFoundObject(StoreEntry *newEntry)
     /** \li If the request has no-cache flag set or some no_cache HACK in operation we
       * 'invalidate' the cached IP entries for this request ???
       */
-    if (r->flags.nocache) {
+    if (r->flags.noCache()) {
 
 #if USE_DNSHELPER
         ipcacheInvalidate(r->GetHost());
@@ -1650,7 +1650,7 @@ clientReplyContext::identifyFoundObject(StoreEntry *newEntry)
         return;
     }
 
-    if (r->flags.nocache) {
+    if (r->flags.noCache()) {
         debugs(85, 3, "clientProcessRequest2: no-cache REFRESH MISS");
         http->storeEntry(NULL);
         http->logType = LOG_TCP_CLIENT_REFRESH_MISS;
diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 9e4547555c..db5200c060 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -588,9 +588,9 @@ ClientRequestContext::hostHeaderVerifyFailed(const char *A, const char *B)
 
         // NP: it is tempting to use 'flags.nocache' but that is all about READing cache data.
         // The problems here are about WRITE for new cache content, which means flags.cachable
-        http->request->flags.cachable = 0; // MUST NOT cache (for now)
+        http->request->flags.setNotCachable(); // MUST NOT cache (for now)
         // XXX: when we have updated the cache key to base on raw-IP + URI this cacheable limit can go.
-        http->request->flags.hierarchical = 0; // MUST NOT pass to peers (for now)
+        http->request->flags.clearHierarchical(); // MUST NOT pass to peers (for now)
         // XXX: when we have sorted out the best way to relay requests properly to peers this hierarchical limit can go.
         http->doCallouts();
         return;
@@ -933,14 +933,14 @@ clientHierarchical(ClientHttpRequest * http)
      * neighbors support private keys
      */
 
-    if (request->flags.ims && !neighbors_do_private_keys)
+    if (request->flags.hasIMS() && !neighbors_do_private_keys)
         return 0;
 
     /*
      * This is incorrect: authenticating requests can be sent via a hierarchy
      * (they can even be cached if the correct headers are set on the reply)
      */
-    if (request->flags.auth)
+    if (request->flags.hasAuth())
         return 0;
 
     if (method == METHOD_TRACE)
@@ -988,7 +988,7 @@ clientCheckPinning(ClientHttpRequest * http)
         if (Comm::IsConnOpen(http_conn->pinning.serverConnection)) {
             if (http_conn->pinning.auth) {
                 request->flags.wantConnectionAuth();
-                request->flags.auth = 1;
+                request->flags.markAuth();
             } else {
                 request->flags.requestConnectionProxyAuth();
             }
@@ -1045,7 +1045,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
     request->ims = req_hdr->getTime(HDR_IF_MODIFIED_SINCE);
 
     if (request->ims > 0)
-        request->flags.ims = 1;
+        request->flags.setIMS();
 
     if (!request->flags.ignoringCacheControl()) {
         if (req_hdr->has(HDR_PRAGMA)) {
@@ -1069,7 +1069,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
         * SP1 or not so all 5.5 versions are treated 'normally').
         */
         if (Config.onoff.ie_refresh) {
-            if (http->flags.accel && request->flags.ims) {
+            if (http->flags.accel && request->flags.hasIMS()) {
                 if ((str = req_hdr->getStr(HDR_USER_AGENT))) {
                     if (strstr(str, "MSIE 5.01") != NULL)
                         no_cache=true;
@@ -1098,7 +1098,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
         else
 #endif
 
-            request->flags.nocache = 1;
+            request->flags.setNocache();
     }
 
     /* ignore range header in non-GETs or non-HEADs */
@@ -1134,12 +1134,12 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
     }
 
     if (req_hdr->has(HDR_AUTHORIZATION))
-        request->flags.auth = 1;
+        request->flags.markAuth();
 
     clientCheckPinning(http);
 
     if (request->login[0] != '\0')
-        request->flags.auth = 1;
+        request->flags.markAuth();
 
     if (req_hdr->has(HDR_VIA)) {
         String s = req_hdr->getList(HDR_VIA);
@@ -1173,17 +1173,17 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
 
 #endif
 
-    request->flags.cachable = http->request->cacheable();
+    request->flags.setCachable(http->request->cacheable());
 
     if (clientHierarchical(http))
-        request->flags.hierarchical = 1;
+        request->flags.setHierarchical();
 
     debugs(85, 5, "clientInterpretRequestHeaders: REQ_NOCACHE = " <<
-           (request->flags.nocache ? "SET" : "NOT SET"));
+           (request->flags.noCache() ? "SET" : "NOT SET"));
     debugs(85, 5, "clientInterpretRequestHeaders: REQ_CACHABLE = " <<
-           (request->flags.cachable ? "SET" : "NOT SET"));
+           (request->flags.isCachable() ? "SET" : "NOT SET"));
     debugs(85, 5, "clientInterpretRequestHeaders: REQ_HIERARCHICAL = " <<
-           (request->flags.hierarchical ? "SET" : "NOT SET"));
+           (request->flags.hierarchical() ? "SET" : "NOT SET"));
 
 }
 
@@ -1293,7 +1293,8 @@ void
 ClientRequestContext::checkNoCacheDone(const allow_t &answer)
 {
     acl_checklist = NULL;
-    http->request->flags.cachable = (answer == ACCESS_ALLOWED);
+    if (answer == ACCESS_ALLOWED)
+        http->request->flags.setCachable();
     http->doCallouts();
 }
 
@@ -1601,7 +1602,7 @@ ClientHttpRequest::doCallouts()
         if (!calloutContext->no_cache_done) {
             calloutContext->no_cache_done = true;
 
-            if (Config.accessList.noCache && request->flags.cachable) {
+            if (Config.accessList.noCache && request->flags.isCachable()) {
                 debugs(83, 3, HERE << "Doing calloutContext->checkNoCache()");
                 calloutContext->checkNoCache();
                 return;
diff --git a/src/forward.cc b/src/forward.cc
index 97de1556b1..db682202d7 100644
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -887,7 +887,7 @@ FwdState::connectDone(const Comm::ConnectionPointer &conn, comm_err_t status, in
     if (rePin) {
         debugs(17, 3, HERE << "repinning " << serverConn);
         request->clientConnectionManager->pinConnection(serverConn,
-                request, serverConn->getPeer(), request->flags.auth);
+                request, serverConn->getPeer(), request->flags.hasAuth());
         request->flags.markPinned();
     }
 
@@ -986,7 +986,7 @@ FwdState::connectStart()
             ++n_tries;
             request->flags.markPinned();
             if (pinned_connection->pinnedAuth())
-                request->flags.auth = 1;
+                request->flags.markAuth();
             comm_add_close_handler(serverConn->fd, fwdServerClosedWrapper, this);
             // the server may close the pinned connection before this request
             pconnRace = racePossible;
diff --git a/src/http.cc b/src/http.cc
index 2dae9d7565..2020cf369b 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -379,7 +379,7 @@ HttpStateData::cacheableReply()
         }
     }
 
-    if (request->flags.auth || request->flags.authSent()) {
+    if (request->flags.hasAuth() || request->flags.authSent()) {
         /*
          * Responses to requests with authorization may be cached
          * only if a Cache-Control: public reply header is present.
@@ -1648,7 +1648,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
      */
     if (!we_do_ranges && request->multipartRangeRequest()) {
         /* don't cache the result */
-        request->flags.cachable = 0;
+        request->flags.setNotCachable();
         /* pretend it's not a range request */
         delete request->range;
         request->range = NULL;
@@ -1987,13 +1987,13 @@ HttpStateData::decideIfWeDoRanges (HttpRequest * request)
 
     int64_t roffLimit = request->getRangeOffsetLimit();
 
-    if (NULL == request->range || !request->flags.cachable
+    if (NULL == request->range || !request->flags.isCachable()
             || request->range->offsetLimitExceeded(roffLimit) || request->flags.connectionAuthWanted())
         result = false;
 
     debugs(11, 8, "decideIfWeDoRanges: range specs: " <<
            request->range << ", cachable: " <<
-           request->flags.cachable << "; we_do_ranges: " << result);
+           request->flags.isCachable() << "; we_do_ranges: " << result);
 
     return result;
 }
diff --git a/src/mime.cc b/src/mime.cc
index 2d14023fd2..d1450061c2 100644
--- a/src/mime.cc
+++ b/src/mime.cc
@@ -455,7 +455,7 @@ MimeIcon::created (StoreEntry *newEntry)
         return;
     }
 
-    flags.cachable = 1;
+    flags.setCachable();
     StoreEntry *e = storeCreateEntry(url,
                                      url,
                                      flags,
diff --git a/src/neighbors.cc b/src/neighbors.cc
index 44932cb292..9ce567da67 100644
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -161,10 +161,10 @@ peerAllowedToUse(const CachePeer * p, HttpRequest * request)
     if (neighborType(p, request) == PEER_SIBLING) {
 #if PEER_MULTICAST_SIBLINGS
         if (p->type == PEER_MULTICAST && p->options.mcast_siblings &&
-                (request->flags.nocache || request->flags.refresh() || request->flags.loopDetect() || request->flags.validationNeeded()))
+                (request->flags.noCache() || request->flags.refresh() || request->flags.loopDetect() || request->flags.validationNeeded()))
             debugs(15, 2, "peerAllowedToUse(" << p->name << ", " << request->GetHost() << ") : multicast-siblings optimization match");
 #endif
-        if (request->flags.nocache)
+        if (request->flags.noCache())
             return false;
 
         if (request->flags.refresh())
@@ -230,7 +230,7 @@ peerWouldBePinged(const CachePeer * p, HttpRequest * request)
     /* the case below seems strange, but can happen if the
      * URL host is on the other side of a firewall */
     if (p->type == PEER_SIBLING)
-        if (!request->flags.hierarchical)
+        if (!request->flags.hierarchical())
             return 0;
 
     if (!peerAllowedToUse(p, request))
@@ -789,7 +789,7 @@ neighborsDigestSelect(HttpRequest * request)
     int p_rtt;
     int i;
 
-    if (!request->flags.hierarchical)
+    if (!request->flags.hierarchical())
         return NULL;
 
     storeKeyPublicByRequest(request);
diff --git a/src/peer_digest.cc b/src/peer_digest.cc
index 18a0922f13..24a021d8be 100644
--- a/src/peer_digest.cc
+++ b/src/peer_digest.cc
@@ -370,7 +370,7 @@ peerDigestRequest(PeerDigest * pd)
 
     pd_last_req_time = squid_curtime;
 
-    req->flags.cachable = 1;
+    req->flags.setCachable();
 
     /* the rest is based on clientProcessExpired() */
     req->flags.setRefresh();
diff --git a/src/peer_select.cc b/src/peer_select.cc
index 6d78c61333..bd48ef4c4a 100644
--- a/src/peer_select.cc
+++ b/src/peer_select.cc
@@ -126,7 +126,7 @@ peerSelectIcpPing(HttpRequest * request, int direct, StoreEntry * entry)
     assert(direct != DIRECT_YES);
     debugs(44, 3, "peerSelectIcpPing: " << entry->url()  );
 
-    if (!request->flags.hierarchical && direct != DIRECT_NO)
+    if (!request->flags.hierarchical() && direct != DIRECT_NO)
         return 0;
 
     if (EBIT_TEST(entry->flags, KEY_PRIVATE) && !neighbors_do_private_keys)
@@ -492,7 +492,7 @@ peerSelectFoo(ps_state * ps)
         if (Config.onoff.prefer_direct)
             peerGetSomeDirect(ps);
 
-        if (request->flags.hierarchical || !Config.onoff.nonhierarchical_direct) {
+        if (request->flags.hierarchical() || !Config.onoff.nonhierarchical_direct) {
             peerGetSomeParent(ps);
             peerGetAllParents(ps);
         }
diff --git a/src/refresh.cc b/src/refresh.cc
index 571e25073d..ac9c7e84a4 100644
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -311,7 +311,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
     if (request && !request->flags.ignoringCacheControl()) {
         HttpHdrCc *cc = request->cache_control;
 
-        if (request->flags.ims && (R->flags.refresh_ims || Config.onoff.refresh_all_ims)) {
+        if (request->flags.hasIMS() && (R->flags.refresh_ims || Config.onoff.refresh_all_ims)) {
             /* The clients no-cache header is changed into a IMS query */
             debugs(22, 3, "refreshCheck: YES: refresh-ims");
             return STALE_FORCED_RELOAD;
@@ -331,7 +331,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
         } else {
             /* The clients no-cache header is not overridden on this request */
             debugs(22, 3, "refreshCheck: YES: client reload");
-            request->flags.nocache = 1;
+            request->flags.setNocache();
             return STALE_FORCED_RELOAD;
         }
 
diff --git a/src/store.cc b/src/store.cc
index 941c2c0054..48537763a8 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -829,12 +829,12 @@ storeCreateEntry(const char *url, const char *log_url, const RequestFlags &flags
     mem = e->mem_obj;
     mem->method = method;
 
-    if (neighbors_do_private_keys || !flags.hierarchical)
+    if (neighbors_do_private_keys || !flags.hierarchical())
         e->setPrivateKey();
     else
         e->setPublicKey();
 
-    if (flags.cachable) {
+    if (flags.isCachable()) {
         EBIT_SET(e->flags, ENTRY_CACHABLE);
         EBIT_CLR(e->flags, RELEASE_REQUEST);
     } else {
diff --git a/src/store_client.cc b/src/store_client.cc
index bd9949dd13..80f7e3d975 100644
--- a/src/store_client.cc
+++ b/src/store_client.cc
@@ -791,7 +791,7 @@ CheckQuickAbort2(StoreEntry * entry)
     assert(mem);
     debugs(90, 3, "CheckQuickAbort2: entry=" << entry << ", mem=" << mem);
 
-    if (mem->request && !mem->request->flags.cachable) {
+    if (mem->request && !mem->request->flags.isCachable()) {
         debugs(90, 3, "CheckQuickAbort2: YES !mem->request->flags.cachable");
         return 1;
     }
diff --git a/src/store_digest.cc b/src/store_digest.cc
index b1133c83ac..b2bd96b01f 100644
--- a/src/store_digest.cc
+++ b/src/store_digest.cc
@@ -392,7 +392,7 @@ storeDigestRewriteStart(void *datanotused)
     debugs(71, 2, "storeDigestRewrite: start rewrite #" << sd_state.rewrite_count + 1);
     /* make new store entry */
     url = internalLocalUri("/squid-internal-periodic/", StoreDigestFileName);
-    flags.cachable = 1;
+    flags.setCachable();
     e = storeCreateEntry(url, url, flags, METHOD_GET);
     assert(e);
     sd_state.rewrite_lock = e;
diff --git a/src/tests/testCoss.cc b/src/tests/testCoss.cc
index d2cabb711f..d546faa3da 100644
--- a/src/tests/testCoss.cc
+++ b/src/tests/testCoss.cc
@@ -190,7 +190,7 @@ testCoss::testCossSearch()
     {
         /* Create "vary" base object */
         RequestFlags flags;
-        flags.cachable = 1;
+        flags.setCachable();
         StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags, METHOD_GET);
         HttpReply *rep = (HttpReply *) pe->getReply();	// bypass const
         rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test", -1, -1, squid_curtime + 100000);
diff --git a/src/tests/testNull.cc b/src/tests/testNull.cc
index 0e2bcf59d8..02d9a89758 100644
--- a/src/tests/testNull.cc
+++ b/src/tests/testNull.cc
@@ -161,7 +161,7 @@ testNull::testNullSearch()
     {
         /* Create "vary" base object */
         RequestFlags flags;
-        flags.cachable = 1;
+        flags.setCachable();
         StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags, METHOD_GET);
         /* We are allowed to do this typecast */
         HttpReply *rep = (HttpReply *) pe->getReply();	// bypass const
diff --git a/src/tests/testRock.cc b/src/tests/testRock.cc
index 9ba5666657..1ce93c8503 100644
--- a/src/tests/testRock.cc
+++ b/src/tests/testRock.cc
@@ -165,7 +165,7 @@ StoreEntry *
 testRock::createEntry(const int i)
 {
     RequestFlags flags;
-    flags.cachable = 1;
+    flags.setCachable();
     char url[64];
     snprintf(url, sizeof(url), "dummy url %i", i);
     url[sizeof(url) - 1] = '\0';
diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc
index 1213414642..06c02a6bfe 100644
--- a/src/tests/testUfs.cc
+++ b/src/tests/testUfs.cc
@@ -142,7 +142,7 @@ testUfs::testUfsSearch()
     {
         /* Create "vary" base object */
         RequestFlags flags;
-        flags.cachable = 1;
+        flags.setCachable();
         StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags, METHOD_GET);
         HttpReply *rep = (HttpReply *) pe->getReply();	// bypass const
         rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test", -1, -1, squid_curtime + 100000);