Fix for valgrind-discovered trunk errors.

During start-up, Valgrind reported many errors with a similar
message: "Use of uninitialised value of size 8...". These errors
were caused by HttpRequestMethod& parameter during private
key generation: it was used as a raw void* memory there. HttpRequestMethod
is a non-POD type and has some "padding" bytes, which are not initialized
by the constructor.

The patch simplifies private key generation by using a simple counter for this,
which is sufficient to create a unique key within a single Squid process.
Except fixing Valgrind errors, this solution saves many CPU cycles wasted on
generating MD5 hashes just to "guarantee" uniqueness within a single process.

diff --git a/src/store.cc b/src/store.cc
index fbd8a7f2a0fe09380d1791dc8892c9ff8b7d96a8..b08f4134ad02392ed945a2321ecf971bbd733d22 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -599,8 +599,6 @@ getKeyCounter(void)
 void
 StoreEntry::setPrivateKey()
 {
-    const cache_key *newkey;
-
     if (key && EBIT_TEST(flags, KEY_PRIVATE))
         return;                 /* is already private */
 
@@ -614,12 +612,9 @@ StoreEntry::setPrivateKey()
         hashDelete();
     }
 
-    if (mem_obj && mem_obj->hasUris()) {
+    if (mem_obj && mem_obj->hasUris())
         mem_obj->id = getKeyCounter();
-        newkey = storeKeyPrivate(mem_obj->storeId(), mem_obj->method, mem_obj->id);
-    } else {
-        newkey = storeKeyPrivate("JUNK", Http::METHOD_NONE, getKeyCounter());
-    }
+    const cache_key *newkey = storeKeyPrivate();
 
     assert(hash_lookup(store_table, newkey) == NULL);
     EBIT_SET(flags, KEY_PRIVATE);

diff --git a/src/store_key_md5.cc b/src/store_key_md5.cc
index 36ba631783b6c505334e48a68d1d337f0bbbfcf7..23c22315c5f7e562ce04b67ee9d75bad8bbfec8f 100644 (file)
--- a/src/store_key_md5.cc
+++ b/src/store_key_md5.cc
@@ -80,18 +80,18 @@ storeKeyHashHash(const void *key, unsigned int n)
 }
 
 const cache_key *
-storeKeyPrivate(const char *url, const HttpRequestMethod& method, int id)
+storeKeyPrivate()
 {
-    static cache_key digest[SQUID_MD5_DIGEST_LENGTH];
-    SquidMD5_CTX M;
-    assert(id > 0);
-    debugs(20, 3, "storeKeyPrivate: " << method << " " << url);
-    SquidMD5Init(&M);
-    SquidMD5Update(&M, (unsigned char *) &id, sizeof(id));
-    SquidMD5Update(&M, (unsigned char *) &method, sizeof(method));
-    SquidMD5Update(&M, (unsigned char *) url, strlen(url));
-    SquidMD5Final(digest, &M);
-    return digest;
+    // only the count field is required
+    // others just simplify searching for keys in a multi-process cache.log
+    static struct {
+        uint64_t count;
+        pid_t pid;
+        int32_t kid;
+    } key = { 0, getpid(), KidIdentifier };
+    assert(sizeof(key) == SQUID_MD5_DIGEST_LENGTH);
+    ++key.count;
+    return reinterpret_cast<cache_key*>(&key);
 }
 
 const cache_key *

diff --git a/src/store_key_md5.h b/src/store_key_md5.h
index 7a2d9c7b5e99f99c7ac3e8ee630ddb2a5575c1b1..27aba0bd42510ab8610bc432c614cd15fb0bcc48 100644 (file)
--- a/src/store_key_md5.h
+++ b/src/store_key_md5.h
@@ -25,7 +25,7 @@ const char *storeKeyText(const cache_key *);
 const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&);
 const cache_key *storeKeyPublicByRequest(HttpRequest *);
 const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const HttpRequestMethod&);
-const cache_key *storeKeyPrivate(const char *, const HttpRequestMethod&, int);
+const cache_key *storeKeyPrivate();
 int storeKeyHashBuckets(int);
 int storeKeyNull(const cache_key *);
 void storeKeyInit(void);