From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sat, 8 Jun 2013 00:56:36 +0000 (-0600)
Subject: Simplified MemObject::write() API.
X-Git-Tag: SQUID_3_5_0_1~444^2~59
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=55759ffbbd3e87ef532266db000bbaebc8388435;p=thirdparty%2Fsquid.git

Simplified MemObject::write() API.

The API required a callback, but the call was always synchronous and the
required callback mechanism could not reliably support an async call anyway.

The method adjusted the buffer offset to become relative to headers rather
than body. While the intent to separate headers from body is noble, none of
the existing caches support that separation, and a different API will be
needed to support it correctly anyway. For now, let's reduce the number of
special cases and offset manipulations.
---

diff --git a/src/MemObject.cc b/src/MemObject.cc
index 81884c0b93..7620d1195e 100644
--- a/src/MemObject.cc
+++ b/src/MemObject.cc
@@ -150,21 +150,17 @@ MemObject::unlinkRequest()
 }
 
 void
-MemObject::write ( StoreIOBuffer writeBuffer, STMCB *callback, void *callbackData)
+MemObject::write(const StoreIOBuffer &writeBuffer)
 {
     PROF_start(MemObject_write);
     debugs(19, 6, "memWrite: offset " << writeBuffer.offset << " len " << writeBuffer.length);
 
-    /* the offset is into the content, not the headers */
-    writeBuffer.offset += (_reply ? _reply->hdr_sz : 0);
-
     /* We don't separate out mime headers yet, so ensure that the first
      * write is at offset 0 - where they start
      */
     assert (data_hdr.endOffset() || writeBuffer.offset == 0);
 
     assert (data_hdr.write (writeBuffer));
-    callback (callbackData, writeBuffer);
     PROF_stop(MemObject_write);
 }
 
diff --git a/src/MemObject.h b/src/MemObject.h
index cee4c9e3e2..5c9af1868b 100644
--- a/src/MemObject.h
+++ b/src/MemObject.h
@@ -63,7 +63,7 @@ public:
     /// replaces construction-time URLs with correct ones; see hidden_mem_obj
     void resetUrls(char const *aUrl, char const *aLog_url);
 
-    void write(StoreIOBuffer, STMCB *, void *);
+    void write(const StoreIOBuffer &buf);
     void unlinkRequest();
     HttpReply const *getReply() const;
     void replaceHttpReply(HttpReply *newrep);
diff --git a/src/store.cc b/src/store.cc
index 24f707fee8..fccf0e01a8 100644
--- a/src/store.cc
+++ b/src/store.cc
@@ -74,8 +74,6 @@
 #include <limits.h>
 #endif
 
-static STMCB storeWriteComplete;
-
 #define REBUILD_TIMESTAMP_DELTA_MAX 2
 
 #define STORE_IN_MEM_BUCKETS            (229)
@@ -869,21 +867,6 @@ StoreEntry::expireNow()
     expires = squid_curtime;
 }
 
-void
-storeWriteComplete (void *data, StoreIOBuffer wroteBuffer)
-{
-    PROF_start(storeWriteComplete);
-    StoreEntry *e = (StoreEntry *)data;
-
-    if (EBIT_TEST(e->flags, DELAY_SENDING)) {
-        PROF_stop(storeWriteComplete);
-        return;
-    }
-
-    e->invokeHandlers();
-    PROF_stop(storeWriteComplete);
-}
-
 void
 StoreEntry::write (StoreIOBuffer writeBuffer)
 {
@@ -892,10 +875,17 @@ StoreEntry::write (StoreIOBuffer writeBuffer)
     PROF_start(StoreEntry_write);
     assert(store_status == STORE_PENDING);
 
+    // XXX: caller uses content offset, but we also store headers
+    if (const HttpReply *reply = mem_obj->getReply())
+        writeBuffer.offset += reply->hdr_sz;
+
     debugs(20, 5, "storeWrite: writing " << writeBuffer.length << " bytes for '" << getMD5Text() << "'");
     PROF_stop(StoreEntry_write);
     storeGetMemSpace(writeBuffer.length);
-    mem_obj->write (writeBuffer, storeWriteComplete, this);
+    mem_obj->write(writeBuffer);
+
+    if (!EBIT_TEST(flags, DELAY_SENDING))
+        invokeHandlers();
 }
 
 /* Append incoming data from a primary server to an entry. */
diff --git a/src/store_client.cc b/src/store_client.cc
index 3c6c4186f1..540b8286cb 100644
--- a/src/store_client.cc
+++ b/src/store_client.cc
@@ -479,12 +479,6 @@ store_client::fileRead()
               this);
 }
 
-static void
-storeClientMemWriteComplete(void *data, StoreIOBuffer wroteBuffer)
-{
-    // Nothin to do here but callback is needed
-}
-
 void
 store_client::readBody(const char *buf, ssize_t len)
 {
@@ -512,15 +506,11 @@ store_client::readBody(const char *buf, ssize_t len)
         // The above may start to free our object so we need to check again
         if (entry->mem_obj->inmem_lo == 0) {
             /* Copy read data back into memory.
-             * but first we need to adjust offset.. some parts of the code
-             * counts offset including headers, some parts count offset as
-             * withing the body.. copyInto is including headers, but the mem
-             * cache expects offset without headers (using negative for headers)
-             * eventually not storing packed headers in memory at all.
+             * copyInto.offset includes headers, which is what mem cache needs
              */
             int64_t mem_offset = entry->mem_obj->endOffset();
             if ((copyInto.offset == mem_offset) || (parsed_header && mem_offset == rep->hdr_sz)) {
-                entry->mem_obj->write(StoreIOBuffer(len, copyInto.offset - rep->hdr_sz, copyInto.data), storeClientMemWriteComplete, this);
+                entry->mem_obj->write(StoreIOBuffer(len, copyInto.offset, copyInto.data));
             }
         }
     }