avoided temporary in HttpHdrCc name-to-id lookup

Started implementing have/set/get/clear combo for all members of HttpHdrCc
Fixed zero-pointer-dereference in http.cc

diff --git a/src/HttpHdrCc.cc b/src/HttpHdrCc.cc
index f06b6e68c370f56da265778b452c46cf7ae49baa..6a9cb7e5855239dbf05d5ccb0fa9ceb15b000519 100644 (file)
--- a/src/HttpHdrCc.cc
+++ b/src/HttpHdrCc.cc
@@ -125,8 +125,7 @@ HttpHdrCc::parse(const String & str)
             nlen = ilen;
 
         /* find type */
-        const StringArea tmpstr(item,nlen);
-        const CcNameToIdMap_t::const_iterator i=CcNameToIdMap.find(tmpstr);
+        const CcNameToIdMap_t::const_iterator i=CcNameToIdMap.find(StringArea(item,nlen));
         if (i==CcNameToIdMap.end())
             type=CC_OTHER;
         else
@@ -139,20 +138,18 @@ HttpHdrCc::parse(const String & str)
                 ++CcAttrs[type].stat.repCount;
                 continue;
             }
-        } else {
-            set(type);
         }
 
-        /* post-processing special cases */
+        /* post-processing, including special cases */
         switch (type) {
 
         case CC_MAX_AGE:
             int32_t ma;
             if (!p || !httpHeaderParseInt(p, &ma)) {
                 debugs(65, 2, "cc: invalid max-age specs near '" << item << "'");
-                setMaxAge(MAX_AGE_UNSET);
+                maxAge(MAX_AGE_UNKNOWN);
             } else {
-                setMaxAge(ma);
+                maxAge(ma);
             }
 
             break;
@@ -161,7 +158,7 @@ HttpHdrCc::parse(const String & str)
 
             if (!p || !httpHeaderParseInt(p, &s_maxage)) {
                 debugs(65, 2, "cc: invalid s-maxage specs near '" << item << "'");
-                setSMaxAge(S_MAXAGE_UNSET);
+                sMaxAge(S_MAXAGE_UNKNOWN);
             }
 
             break;
@@ -179,7 +176,7 @@ HttpHdrCc::parse(const String & str)
 
             if (!p || !httpHeaderParseInt(p, &min_fresh)) {
                 debugs(65, 2, "cc: invalid min-fresh specs near '" << item << "'");
-                setMinFresh(MIN_FRESH_UNSET);
+                setMinFresh(MIN_FRESH_UNKNOWN);
             }
 
             break;
@@ -187,7 +184,7 @@ HttpHdrCc::parse(const String & str)
         case CC_STALE_IF_ERROR:
             if (!p || !httpHeaderParseInt(p, &stale_if_error)) {
                 debugs(65, 2, "cc: invalid stale-if-error specs near '" << item << "'");
-                setStaleIfError(STALE_IF_ERROR_UNSET);
+                setStaleIfError(STALE_IF_ERROR_UNKNOWN);
             }
             break;
 
@@ -201,6 +198,8 @@ HttpHdrCc::parse(const String & str)
             break;
 
         default:
+
+            set(type);
             /* note that we ignore most of '=' specs (RFCVIOLATION) */
             break;
         }
@@ -225,10 +224,10 @@ httpHdrCcPackInto(const HttpHdrCc * cc, Packer * p)
             /* handle options with values */
 
             if (flag == CC_MAX_AGE)
-                packerPrintf(p, "=%d", (int) cc->getMaxAge());
+                packerPrintf(p, "=%d", (int) cc->maxAge());
 
             if (flag == CC_S_MAXAGE)
-                packerPrintf(p, "=%d", (int) cc->getSMaxAge());
+                packerPrintf(p, "=%d", (int) cc->sMaxAge());
 
             if (flag == CC_MAX_STALE && cc->getMaxStale() >= 0)
                 packerPrintf(p, "=%d", (int) cc->getMaxStale());

diff --git a/src/HttpHdrCc.cci b/src/HttpHdrCc.cci
index adbb0bcf772ffc222b6d49cd172d1f16af436be8..861b83698f38c8226f7a3e0f614fd467fdfa2600 100644 (file)
--- a/src/HttpHdrCc.cci
+++ b/src/HttpHdrCc.cci
@@ -32,38 +32,19 @@
 
 
 void
-HttpHdrCc::setMaxAge(int max_age_)
+HttpHdrCc::sMaxAge(int32_t s_maxage)
 {
-    /* keeping the mask in sync is needed e.g. for the packer functions */
-    if (max_age_ >= 0) {
-        set(CC_MAX_AGE);
-        max_age = max_age_;
-    } else {
-        set(CC_MAX_AGE,false);
-        max_age=MAX_AGE_UNSET;
-    }
-}
-
-int32_t
-HttpHdrCc::getMaxAge() const
-{
-    return max_age;
-}
-
-void
-HttpHdrCc::setSMaxAge(int32_t s_maxage)
-{
-    if (s_maxage >= 0) {
-        set(CC_S_MAXAGE);
+    if (s_maxage>=0) {
+        setMask(CC_S_MAXAGE);
         this->s_maxage=s_maxage;
     } else {
-        set(CC_S_MAXAGE,false);
-        this->s_maxage=S_MAXAGE_UNSET;
+        setMask(CC_S_MAXAGE,false);
+        this->s_maxage=S_MAXAGE_UNKNOWN;
     }
 }
 
 int32_t
-HttpHdrCc::getSMaxAge() const
+HttpHdrCc::sMaxAge() const
 {
     return s_maxage;
 }
@@ -72,11 +53,11 @@ void
 HttpHdrCc::setMaxStale(int32_t max_stale)
 {
     if (max_stale>=0 || max_stale==MAX_STALE_ALWAYS) {
-        set(CC_MAX_STALE);
+        setMask(CC_MAX_STALE);
         this->max_stale=max_stale;
     } else {
-        set(CC_MAX_STALE,false);
-        this->max_stale=MAX_STALE_UNSET;
+        setMask(CC_MAX_STALE,false);
+        this->max_stale=MAX_STALE_UNKNOWN;
     }
 }
 int32_t
@@ -89,11 +70,11 @@ void
 HttpHdrCc::setStaleIfError(int32_t stale_if_error)
 {
     if (stale_if_error >= 0) {
-        set(CC_STALE_IF_ERROR);
+        setMask(CC_STALE_IF_ERROR);
         this->stale_if_error=stale_if_error;
     } else {
-        set(CC_STALE_IF_ERROR,false);
-        this->stale_if_error=STALE_IF_ERROR_UNSET;
+        setMask(CC_STALE_IF_ERROR,false);
+        this->stale_if_error=STALE_IF_ERROR_UNKNOWN;
     }
 }
 
@@ -107,11 +88,11 @@ void
 HttpHdrCc::setMinFresh(int32_t min_fresh)
 {
     if (min_fresh >= 0) {
-        set(CC_MIN_FRESH);
+        setMask(CC_MIN_FRESH);
         this->min_fresh=min_fresh;
     } else {
-        set(CC_MIN_FRESH,false);
-        this->min_fresh=MIN_FRESH_UNSET;
+        setMask(CC_MIN_FRESH,false);
+        this->min_fresh=MIN_FRESH_UNKNOWN;
     }
 }
 
@@ -132,6 +113,15 @@ void
 HttpHdrCc::set(http_hdr_cc_type id, bool newval)
 {
     assert(id>=CC_PUBLIC && id < CC_ENUM_END);
+    assert(id!=CC_MAX_AGE && id != CC_S_MAXAGE &&
+            id != CC_MAX_STALE && id != CC_STALE_IF_ERROR &&
+            id != CC_MIN_FRESH);
+    setMask(id,newval);
+}
+
+void
+HttpHdrCc::setMask(http_hdr_cc_type id, bool newval)
+{
     if (newval)
         EBIT_SET(mask,id);
     else

diff --git a/src/HttpHdrCc.h b/src/HttpHdrCc.h
index 4a0ca70870a6a1d8cac2452b81dfc12012cc46bb..18ad00d296dc86e55201a63f7a6c92c05b112bb6 100644 (file)
--- a/src/HttpHdrCc.h
+++ b/src/HttpHdrCc.h
@@ -44,17 +44,17 @@ class HttpHdrCc
 {
 
 public:
-       static const int32_t MAX_AGE_UNSET=-1; //max-age is unset
-       static const int32_t S_MAXAGE_UNSET=-1; //s-maxage is unset
-       static const int32_t MAX_STALE_UNSET=-1; //max-stale is unset
+       static const int32_t MAX_AGE_UNKNOWN=-1; //max-age is unset
+       static const int32_t S_MAXAGE_UNKNOWN=-1; //s-maxage is unset
+       static const int32_t MAX_STALE_UNKNOWN=-1; //max-stale is unset
        static const int32_t MAX_STALE_ALWAYS=-2; //max-stale is set to no value
-       static const int32_t STALE_IF_ERROR_UNSET=-1; //stale_if_error is unset
-       static const int32_t MIN_FRESH_UNSET=-1; //min_fresh is unset
+       static const int32_t STALE_IF_ERROR_UNKNOWN=-1; //stale_if_error is unset
+       static const int32_t MIN_FRESH_UNKNOWN=-1; //min_fresh is unset
 
     HttpHdrCc() :
-            mask(0), max_age(MAX_AGE_UNSET), s_maxage(S_MAXAGE_UNSET),
-            max_stale(MAX_STALE_UNSET), stale_if_error(STALE_IF_ERROR_UNSET),
-            min_fresh(MIN_FRESH_UNSET) {}
+            mask(0), max_age(MAX_AGE_UNKNOWN), s_maxage(S_MAXAGE_UNKNOWN),
+            max_stale(MAX_STALE_UNKNOWN), stale_if_error(STALE_IF_ERROR_UNKNOWN),
+            min_fresh(MIN_FRESH_UNKNOWN) {}
 
     /// reset data-members to default state
     void clear();
@@ -62,31 +62,82 @@ public:
     /// parse a header-string and fill in appropriate values.
     bool parse(const String & s);
 
-    /// set an attribute value or clear it (by supplying false as the second argument)
-    _SQUID_INLINE_ void set(http_hdr_cc_type id, bool newval=true);
-    /// check whether the attribute value supplied by id is set
-    _SQUID_INLINE_ bool isSet(http_hdr_cc_type id) const;
-
-    /// max-age setter. Clear by setting to MAX_AGE_UNSET
-    _SQUID_INLINE_ void setMaxAge(int32_t max_age);
-    _SQUID_INLINE_ int32_t getMaxAge() const;
-
-    /// s-maxage setter. Clear by setting to S_MAXAGE_UNSET
-    _SQUID_INLINE_ void setSMaxAge(int32_t s_maxage);
-    _SQUID_INLINE_ int32_t getSMaxAge() const;
-
-    /// max-stale setter. Clear by setting to MAX_STALE_UNSET
+    //manipulation for Cache-Control: XXX header
+    //inline bool haveXXX() const {return isSet();}
+    //inline bool XXX() const {return isSet();}
+    //inline void XXX(bool newval) {setMask(,newval);}
+    //inline void clearXXX() {setMask(,false);}
+
+    //manipulation for Cache-Control: public header
+    inline bool havePublic() const {return isSet(CC_PUBLIC);}
+    inline bool getPublic() const {return isSet(CC_PUBLIC);}
+    inline void setPublic(bool newval) {setMask(CC_PUBLIC,newval);}
+    inline void clearPublic() {setMask(CC_PUBLIC,false);}
+
+    //manipulation for Cache-Control: private header
+    inline bool havePrivate() const {return isSet(CC_PRIVATE);}
+    inline bool getPrivate() const {return isSet(CC_PRIVATE);}
+    inline void setPrivate(bool newval) {setMask(CC_PRIVATE,newval);}
+    inline void clearPrivate() {setMask(CC_PRIVATE,false);}
+
+    //manipulation for Cache-Control: no-cache header
+    inline bool haveNoCache() const {return isSet(CC_NO_CACHE);}
+    inline bool noCache() const {return isSet(CC_NO_CACHE);}
+    inline void noCache(bool newval) {setMask(CC_NO_CACHE,newval);}
+    inline void clearNoCache() {setMask(CC_NO_CACHE,false);}
+
+    //manipulation for Cache-Control: no-store header
+    inline bool haveNoStore() const {return isSet(CC_NO_STORE);}
+    inline bool noStore() const {return isSet(CC_NO_STORE);}
+    inline void noStore(bool newval) {setMask(CC_NO_STORE,newval);}
+    inline void clearNoStore() {setMask(CC_NO_STORE,false);}
+
+    //manipulation for Cache-Control: no-transform header
+    inline bool haveNoTransform() const {return isSet(CC_NO_TRANSFORM);}
+    inline bool noTransform() const {return isSet(CC_NO_TRANSFORM);}
+    inline void noTransform(bool newval) {setMask(CC_NO_TRANSFORM,newval);}
+    inline void clearNoTransform() {setMask(CC_NO_TRANSFORM,false);}
+
+    //manipulation for Cache-Control: must-revalidate header
+    inline bool haveMustRevalidate() const {return isSet(CC_MUST_REVALIDATE);}
+    inline bool mustRevalidate() const {return isSet(CC_MUST_REVALIDATE);}
+    inline void mustRevalidate(bool newval) {setMask(CC_MUST_REVALIDATE,newval);}
+    inline void clearMustRevalidate() {setMask(CC_MUST_REVALIDATE,false);}
+
+    //manipulation for Cache-Control: proxy-revalidate header
+    inline bool haveProxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);}
+    inline bool proxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);}
+    inline void proxyRevalidate(bool newval) {setMask(CC_PROXY_REVALIDATE,newval);}
+    inline void clearProxyRevalidate() {setMask(CC_PROXY_REVALIDATE,false);}
+
+    //manipulation for Cache-Control: max-age header
+    inline bool haveMaxAge() const {return isSet(CC_MAX_AGE);}
+    inline int32_t maxAge() const { return max_age;}
+    inline void maxAge(int32_t max_age) {this->max_age = max_age; setMask(CC_MAX_AGE); }
+    inline void clearMaxAge() {max_age=MAX_AGE_UNKNOWN; setMask(CC_MAX_AGE,false);}
+
+
+    /// s-maxage setter. Clear by setting to S_MAXAGE_UNKNOWN
+    _SQUID_INLINE_ void sMaxAge(int32_t s_maxage);
+    _SQUID_INLINE_ int32_t sMaxAge() const;
+
+    /// max-stale setter. Clear by setting to MAX_STALE_UNKNOWN
     _SQUID_INLINE_ void setMaxStale(int32_t max_stale);
     _SQUID_INLINE_ int32_t getMaxStale() const;
 
-    /// stale-if-error setter. Clear by setting to STALE_IF_ERROR_UNSET
+    /// stale-if-error setter. Clear by setting to STALE_IF_ERROR_UNKNOWN
     _SQUID_INLINE_ void setStaleIfError(int32_t stale_if_error);
     _SQUID_INLINE_ int32_t getStaleIfError() const;
 
-    /// min-fresh setter. Clear by setting to MIN_FRESH_UNSET
+    /// min-fresh setter. Clear by setting to MIN_FRESH_UNKNOWN
     _SQUID_INLINE_ void setMinFresh(int32_t min_fresh);
     _SQUID_INLINE_ int32_t getMinFresh() const;
 
+    /// set an attribute value or clear it (by supplying false as the second argument)
+    _SQUID_INLINE_ void set(http_hdr_cc_type id, bool newval=true);
+    /// check whether the attribute value supplied by id is set
+    _SQUID_INLINE_ bool isSet(http_hdr_cc_type id) const;
+
     MEMPROXY_CLASS(HttpHdrCc);
 
     /** bit-mask representing what header values are set among those
@@ -101,6 +152,9 @@ private:
     int32_t max_stale;
     int32_t stale_if_error;
     int32_t min_fresh;
+    /// low-level part of the public set method, performs no checks
+    _SQUID_INLINE_ void setMask(http_hdr_cc_type id, bool newval=true);
+
 public:
     /**comma-separated representation of the header values which were
      * received but are not recognized.

diff --git a/src/HttpReply.cc b/src/HttpReply.cc
index 6ff6694561961878bad40b07dca4e203cf31914f..1559c1d2061e7a03d2f0ec7661e3955894005cb1 100644 (file)
--- a/src/HttpReply.cc
+++ b/src/HttpReply.cc
@@ -331,21 +331,21 @@ HttpReply::hdrExpirationTime()
 
     if (cache_control) {
         if (date >= 0) {
-            if (cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET)
-                return date + cache_control->getSMaxAge();
+            if (cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN)
+                return date + cache_control->sMaxAge();
 
-            if (cache_control->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET)
-                return date + cache_control->getMaxAge();
+            if (cache_control->isSet(CC_MAX_AGE))
+                return date + cache_control->maxAge();
         } else {
             /*
              * Conservatively handle the case when we have a max-age
              * header, but no Date for reference?
              */
 
-            if (cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET)
+            if (cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN)
                 return squid_curtime;
 
-            if (cache_control->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET)
+            if (cache_control->isSet(CC_MAX_AGE))
                 return squid_curtime;
         }
     }

diff --git a/src/base/StringArea.h b/src/base/StringArea.h
index 1ffda12716a501e02c8a323740a46e4415db8d2e..719bd1f8501f1bd6eb7be11b0a9899790ef01603 100644 (file)
--- a/src/base/StringArea.h
+++ b/src/base/StringArea.h
@@ -47,13 +47,20 @@
  */
 class StringArea {
     public:
+    /// build a StringArea by explicitly assigning pointed-to area and and length
     StringArea(const char * ptr, size_t len): theStart(ptr), theLen(len) {}
-    bool operator==(const StringArea &s) const { return theLen==s.theLen && strncmp(theStart,s.theStart,theLen)==0; }
+    bool operator==(const StringArea &s) const { return theLen==s.theLen && memcmp(theStart,s.theStart,theLen)==0; }
     bool operator!=(const StringArea &s) const { return !operator==(s); }
-    bool operator< ( const StringArea &s) const { return theLen < s.theLen || strncmp(theStart,s.theStart,theLen) < 0; }
+    bool operator< ( const StringArea &s) const {
+        return (theLen < s.theLen || (theLen == s.theLen && memcmp(theStart,s.theStart,theLen) < 0)) ; }
 
     private:
+    // default constructor is disabled
+    StringArea();
+
+    /// pointed to the externally-managed memory area
     const char *theStart;
+    /// length of the string
     size_t theLen;
 };
 

diff --git a/src/http.cc b/src/http.cc
index 96c1f3690a68f1d95d5cc1b5c909dbe87b63f031..e826ce3ae4732b1a289ade9ae55e0458f4ef14cd 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -357,7 +357,7 @@ HttpStateData::cacheableReply()
             !REFRESH_OVERRIDE(ignore_no_store))
         return 0;
 
-    if (!ignoreCacheControl) {
+    if (!ignoreCacheControl && request->cache_control != NULL) {
         const HttpHdrCc* cc=request->cache_control;
         if (cc->isSet(CC_PRIVATE)) {
             if (!REFRESH_OVERRIDE(ignore_private))
@@ -928,7 +928,7 @@ no_cache:
     if (!ignoreCacheControl && rep->cache_control) {
         if (rep->cache_control->isSet(CC_PROXY_REVALIDATE) ||
                 rep->cache_control->isSet(CC_MUST_REVALIDATE) ||
-                rep->cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET
+                rep->cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN
                 )
             EBIT_SET(entry->flags, ENTRY_REVALIDATE);
     }
@@ -1771,10 +1771,10 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
 #endif
 
         /* Add max-age only without no-cache */
-        if (cc->getMaxAge()==HttpHdrCc::MAX_AGE_UNSET && !cc->isSet(CC_NO_CACHE)) {
+        if (cc->isSet(CC_MAX_AGE) && !cc->isSet(CC_NO_CACHE)) {
             const char *url =
                 entry ? entry->url() : urlCanonical(request);
-            cc->setMaxAge(getMaxAge(url));
+            cc->maxAge(getMaxAge(url));
 
         }
 

diff --git a/src/mime.cc b/src/mime.cc
index 1d8c4b485bc6e8d7e13ff6c6f0b8381b346b81f3..7c7af825c48a8faf883f6dea13e27b2309ed7e72 100644 (file)
--- a/src/mime.cc
+++ b/src/mime.cc
@@ -465,7 +465,7 @@ MimeIcon::created (StoreEntry *newEntry)
 
     reply->cache_control = new HttpHdrCc();
 
-    reply->cache_control->setMaxAge(86400);
+    reply->cache_control->maxAge(86400);
 
     reply->header.putCc(reply->cache_control);
 

diff --git a/src/refresh.cc b/src/refresh.cc
index af4fdc7afa62f72237631b61628c7f1ee4959fe4..97450d5673cc56e2b3f70224649f9471299c8cb3 100644 (file)
--- a/src/refresh.cc
+++ b/src/refresh.cc
@@ -268,7 +268,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
 
     if (request && !request->flags.ignore_cc) {
         const HttpHdrCc *const cc = request->cache_control;
-        if (cc && cc->getMinFresh()!=HttpHdrCc::MIN_FRESH_UNSET) {
+        if (cc && cc->getMinFresh()!=HttpHdrCc::MIN_FRESH_UNKNOWN) {
             const int32_t minFresh=cc->getMinFresh();
             debugs(22, 3, "\tage + min-fresh:\t" << age << " + " <<
                        minFresh << " = " << age + minFresh);
@@ -288,7 +288,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
 
     // stale-if-error requires any failure be passed thru when its period is over.
     if (request && entry->mem_obj && entry->mem_obj->getReply() && entry->mem_obj->getReply()->cache_control &&
-               entry->mem_obj->getReply()->cache_control->getStaleIfError() != HttpHdrCc::STALE_IF_ERROR_UNSET &&
+               entry->mem_obj->getReply()->cache_control->getStaleIfError() != HttpHdrCc::STALE_IF_ERROR_UNKNOWN &&
             entry->mem_obj->getReply()->cache_control->getStaleIfError() < staleness) {
 
         debugs(22, 3, "refreshCheck: stale-if-error period expired.");
@@ -336,19 +336,19 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta)
 
 #endif
         if (NULL != cc) {
-            if (cc->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET) {
+            if (cc->isSet(CC_MAX_AGE)) {
 #if USE_HTTP_VIOLATIONS
-                if (R->flags.ignore_reload && cc->getMaxAge() == 0) {
+                if (R->flags.ignore_reload && cc->maxAge() == 0) {
                     debugs(22, 3, "refreshCheck: MAYBE: client-max-age = 0 and ignore-reload");
                 } else
 #endif
                 {
-                    if (cc->getMaxAge() == 0) {
+                    if (cc->maxAge() == 0) {
                         debugs(22, 3, "refreshCheck: YES: client-max-age = 0");
                         return STALE_EXCEEDS_REQUEST_MAX_AGE_VALUE;
                     }
 
-                    if (age > cc->getMaxAge()) {
+                    if (age > cc->maxAge()) {
                         debugs(22, 3, "refreshCheck: YES: age > client-max-age");
                         return STALE_EXCEEDS_REQUEST_MAX_AGE_VALUE;
                     }