Bug 1320894 - Fixed serialization of cache index, r=valentin
authorMichal Novotny <michal.novotny@gmail.com>
Fri, 02 Dec 2016 16:21:35 +0100
changeset 325158 331ca1a40ca24f3f8dfeece611f05754e2735a5e
parent 325157 d614a623d6817e0055d184c49876804e39c5a514
child 325159 b7738f546106aa38f9a7f1927c9211a77137979a
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersvalentin
bugs1320894
milestone53.0a1
Bug 1320894 - Fixed serialization of cache index, r=valentin
netwerk/cache2/CacheIndex.cpp
netwerk/cache2/CacheIndex.h
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -22,17 +22,17 @@
 #include <algorithm>
 #include "mozilla/Telemetry.h"
 #include "mozilla/Unused.h"
 
 
 #define kMinUnwrittenChanges   300
 #define kMinDumpInterval       20000 // in milliseconds
 #define kMaxBufSize            16384
-#define kIndexVersion          0x00000002
+#define kIndexVersion          0x00000003
 #define kUpdateIndexStartDelay 50000 // in milliseconds
 
 #define INDEX_NAME      "index"
 #define TEMP_INDEX_NAME "index.tmp"
 #define JOURNAL_NAME    "index.log"
 
 namespace mozilla {
 namespace net {
@@ -1644,23 +1644,28 @@ CacheIndex::WriteIndexToDisk()
     return;
   }
 
   // Write index header to a buffer, it will be written to disk together with
   // records in WriteRecords() once we open the file successfully.
   AllocBuffer();
   mRWHash = new CacheHash();
 
-  CacheIndexHeader *hdr = reinterpret_cast<CacheIndexHeader *>(mRWBuf);
-  NetworkEndian::writeUint32(&hdr->mVersion, kIndexVersion);
-  NetworkEndian::writeUint32(&hdr->mTimeStamp,
+  mRWBufPos = 0;
+  // index version
+  NetworkEndian::writeUint32(mRWBuf + mRWBufPos, kIndexVersion);
+  mRWBufPos += sizeof(uint32_t);
+  // timestamp
+  NetworkEndian::writeUint32(mRWBuf + mRWBufPos,
                              static_cast<uint32_t>(PR_Now() / PR_USEC_PER_SEC));
-  NetworkEndian::writeUint32(&hdr->mIsDirty, 1);
-
-  mRWBufPos = sizeof(CacheIndexHeader);
+  mRWBufPos += sizeof(uint32_t);
+  // dirty flag
+  NetworkEndian::writeUint32(mRWBuf + mRWBufPos, 1);
+  mRWBufPos += sizeof(uint32_t);
+
   mSkipEntries = 0;
 }
 
 void
 CacheIndex::WriteRecords()
 {
   LOG(("CacheIndex::WriteRecords()"));
 
@@ -2005,34 +2010,29 @@ CacheIndex::WriteLogToDisk()
 
   rv = wlh.Finish();
   PR_Close(fd);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = indexFile->OpenNSPRFileDesc(PR_RDWR, 0600, &fd);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  CacheIndexHeader header;
-  int32_t bytesRead = PR_Read(fd, &header, sizeof(CacheIndexHeader));
-  if (bytesRead != sizeof(CacheIndexHeader)) {
-    PR_Close(fd);
-    return NS_ERROR_FAILURE;
-  }
-
-  NetworkEndian::writeUint32(&header.mIsDirty, 0);
-
-  int64_t offset = PR_Seek64(fd, 0, PR_SEEK_SET);
+  // Seek to dirty flag in the index header and clear it.
+  static_assert(2 * sizeof(uint32_t) == offsetof(CacheIndexHeader, mIsDirty),
+                "Unexpected offset of CacheIndexHeader::mIsDirty");
+  int64_t offset = PR_Seek64(fd, 2 * sizeof(uint32_t), PR_SEEK_SET);
   if (offset == -1) {
     PR_Close(fd);
     return NS_ERROR_FAILURE;
   }
 
-  int32_t bytesWritten = PR_Write(fd, &header, sizeof(CacheIndexHeader));
+  uint32_t isDirty = 0;
+  int32_t bytesWritten = PR_Write(fd, &isDirty, sizeof(isDirty));
   PR_Close(fd);
-  if (bytesWritten != sizeof(CacheIndexHeader)) {
+  if (bytesWritten != sizeof(isDirty)) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
 void
 CacheIndex::ReadIndexFromDisk()
@@ -2135,51 +2135,47 @@ CacheIndex::ParseRecords()
 
   MOZ_ASSERT(!mRWPending);
 
   uint32_t entryCnt = (mIndexHandle->FileSize() - sizeof(CacheIndexHeader) -
                      sizeof(CacheHash::Hash32_t)) / sizeof(CacheIndexRecord);
   uint32_t pos = 0;
 
   if (!mSkipEntries) {
-    CacheIndexHeader *hdr = reinterpret_cast<CacheIndexHeader *>(
-                              moz_xmalloc(sizeof(CacheIndexHeader)));
-    memcpy(hdr, mRWBuf, sizeof(CacheIndexHeader));
-
-    if (NetworkEndian::readUint32(&hdr->mVersion) != kIndexVersion) {
-      free(hdr);
+    if (NetworkEndian::readUint32(mRWBuf + pos) != kIndexVersion) {
       FinishRead(false);
       return;
     }
-
-    mIndexTimeStamp = NetworkEndian::readUint32(&hdr->mTimeStamp);
-
-    if (NetworkEndian::readUint32(&hdr->mIsDirty)) {
+    pos += sizeof(uint32_t);
+
+    mIndexTimeStamp = NetworkEndian::readUint32(mRWBuf + pos);
+    pos += sizeof(uint32_t);
+
+    if (NetworkEndian::readUint32(mRWBuf + pos)) {
       if (mJournalHandle) {
         CacheFileIOManager::DoomFile(mJournalHandle, nullptr);
         mJournalHandle = nullptr;
       }
-      free(hdr);
     } else {
-      NetworkEndian::writeUint32(&hdr->mIsDirty, 1);
+      uint32_t * isDirty = reinterpret_cast<uint32_t *>(
+                             moz_xmalloc(sizeof(uint32_t)));
+      NetworkEndian::writeUint32(isDirty, 1);
 
       // Mark index dirty. The buffer is freed by CacheFileIOManager when
       // nullptr is passed as the listener and the call doesn't fail
       // synchronously.
-      rv = CacheFileIOManager::Write(mIndexHandle, 0,
-                                     reinterpret_cast<char *>(hdr),
-                                     sizeof(CacheIndexHeader), true, false,
-                                     nullptr);
+      rv = CacheFileIOManager::Write(mIndexHandle, 2 * sizeof(uint32_t),
+                                     reinterpret_cast<char *>(isDirty),
+                                     sizeof(uint32_t), true, false, nullptr);
       if (NS_FAILED(rv)) {
         // This is not fatal, just free the memory
-        free(hdr);
+        free(isDirty);
       }
     }
-
-    pos += sizeof(CacheIndexHeader);
+    pos += sizeof(uint32_t);
   }
 
   uint32_t hashOffset = pos;
 
   while (pos + sizeof(CacheIndexRecord) <= mRWBufPos &&
          mSkipEntries != entryCnt) {
     CacheIndexRecord *rec = reinterpret_cast<CacheIndexRecord *>(mRWBuf + pos);
     CacheIndexEntry tmpEntry(&rec->mHash);
@@ -2309,18 +2305,17 @@ CacheIndex::ParseJournal()
 
   uint32_t entryCnt = (mJournalHandle->FileSize() -
                        sizeof(CacheHash::Hash32_t)) / sizeof(CacheIndexRecord);
 
   uint32_t pos = 0;
 
   while (pos + sizeof(CacheIndexRecord) <= mRWBufPos &&
          mSkipEntries != entryCnt) {
-    CacheIndexRecord *rec = reinterpret_cast<CacheIndexRecord *>(mRWBuf + pos);
-    CacheIndexEntry tmpEntry(&rec->mHash);
+    CacheIndexEntry tmpEntry(reinterpret_cast<SHA1Sum::Hash *>(mRWBuf + pos));
     tmpEntry.ReadFromBuf(mRWBuf + pos);
 
     CacheIndexEntry *entry = mTmpJournal.PutEntry(*tmpEntry.Hash());
     *entry = tmpEntry;
 
     if (entry->IsDirty() || entry->IsFresh()) {
       LOG(("CacheIndex::ParseJournal() - Invalid entry found in journal, "
            "ignoring whole journal [dirty=%d, fresh=%d]", entry->IsDirty(),
--- a/netwerk/cache2/CacheIndex.h
+++ b/netwerk/cache2/CacheIndex.h
@@ -51,42 +51,53 @@ typedef struct {
 
   // We set this flag as soon as possible after parsing index during startup
   // and clean it after we write journal to disk during shutdown. We ignore the
   // journal and start update process whenever this flag is set during index
   // parsing.
   uint32_t mIsDirty;
 } CacheIndexHeader;
 
+static_assert(
+  sizeof(CacheIndexHeader::mVersion) + sizeof(CacheIndexHeader::mTimeStamp) +
+  sizeof(CacheIndexHeader::mIsDirty) == sizeof(CacheIndexHeader),
+  "Unexpected sizeof(CacheIndexHeader)!");
+
 struct CacheIndexRecord {
   SHA1Sum::Hash   mHash;
   uint32_t        mFrecency;
+  OriginAttrsHash mOriginAttrsHash;
   uint32_t        mExpirationTime;
-  OriginAttrsHash mOriginAttrsHash;
 
   /*
    *    1000 0000 0000 0000 0000 0000 0000 0000 : initialized
    *    0100 0000 0000 0000 0000 0000 0000 0000 : anonymous
    *    0010 0000 0000 0000 0000 0000 0000 0000 : removed
    *    0001 0000 0000 0000 0000 0000 0000 0000 : dirty
    *    0000 1000 0000 0000 0000 0000 0000 0000 : fresh
    *    0000 0100 0000 0000 0000 0000 0000 0000 : pinned
    *    0000 0011 0000 0000 0000 0000 0000 0000 : reserved
    *    0000 0000 1111 1111 1111 1111 1111 1111 : file size (in kB)
    */
-  uint32_t      mFlags;
+  uint32_t        mFlags;
 
   CacheIndexRecord()
     : mFrecency(0)
+    , mOriginAttrsHash(0)
     , mExpirationTime(nsICacheEntry::NO_EXPIRATION_TIME)
-    , mOriginAttrsHash(0)
     , mFlags(0)
   {}
 };
 
+static_assert(
+  sizeof(CacheIndexRecord::mHash) + sizeof(CacheIndexRecord::mFrecency) +
+  sizeof(CacheIndexRecord::mOriginAttrsHash) + sizeof(CacheIndexRecord::mExpirationTime) +
+  sizeof(CacheIndexRecord::mFlags) == sizeof(CacheIndexRecord),
+  "Unexpected sizeof(CacheIndexRecord)!");
+
 class CacheIndexEntry : public PLDHashEntryHdr
 {
 public:
   typedef const SHA1Sum::Hash& KeyType;
   typedef const SHA1Sum::Hash* KeyTypePointer;
 
   explicit CacheIndexEntry(KeyTypePointer aKey)
   {
@@ -216,46 +227,34 @@ public:
   static uint32_t IsPinned(CacheIndexRecord *aRec)
   {
     return aRec->mFlags & kPinnedMask;
   }
   bool     IsFileEmpty() const { return GetFileSize() == 0; }
 
   void WriteToBuf(void *aBuf)
   {
-    CacheIndexRecord *dst = reinterpret_cast<CacheIndexRecord *>(aBuf);
-
-    // Copy the whole record to the buffer.
-    memcpy(aBuf, mRec, sizeof(CacheIndexRecord));
-
+    uint8_t* ptr = static_cast<uint8_t*>(aBuf);
+    memcpy(ptr, mRec->mHash, sizeof(SHA1Sum::Hash)); ptr += sizeof(SHA1Sum::Hash);
+    NetworkEndian::writeUint32(ptr, mRec->mFrecency); ptr += sizeof(uint32_t);
+    NetworkEndian::writeUint64(ptr, mRec->mOriginAttrsHash); ptr += sizeof(uint64_t);
+    NetworkEndian::writeUint32(ptr, mRec->mExpirationTime); ptr += sizeof(uint32_t);
     // Dirty and fresh flags should never go to disk, since they make sense only
     // during current session.
-    dst->mFlags &= ~kDirtyMask;
-    dst->mFlags &= ~kFreshMask;
-
-#if defined(IS_LITTLE_ENDIAN)
-    // Data in the buffer are in machine byte order and we want them in network
-    // byte order.
-    NetworkEndian::writeUint32(&dst->mFrecency, dst->mFrecency);
-    NetworkEndian::writeUint32(&dst->mExpirationTime, dst->mExpirationTime);
-    NetworkEndian::writeUint64(&dst->mOriginAttrsHash, dst->mOriginAttrsHash);
-    NetworkEndian::writeUint32(&dst->mFlags, dst->mFlags);
-#endif
+    NetworkEndian::writeUint32(ptr, mRec->mFlags & ~(kDirtyMask | kFreshMask));
   }
 
   void ReadFromBuf(void *aBuf)
   {
-    CacheIndexRecord *src= reinterpret_cast<CacheIndexRecord *>(aBuf);
-    MOZ_ASSERT(memcmp(&mRec->mHash, &src->mHash,
-               sizeof(SHA1Sum::Hash)) == 0);
-
-    mRec->mFrecency = NetworkEndian::readUint32(&src->mFrecency);
-    mRec->mExpirationTime = NetworkEndian::readUint32(&src->mExpirationTime);
-    mRec->mOriginAttrsHash = NetworkEndian::readUint64(&src->mOriginAttrsHash);
-    mRec->mFlags = NetworkEndian::readUint32(&src->mFlags);
+    const uint8_t* ptr = static_cast<const uint8_t*>(aBuf);
+    MOZ_ASSERT(memcmp(&mRec->mHash, ptr, sizeof(SHA1Sum::Hash)) == 0); ptr += sizeof(SHA1Sum::Hash);
+    mRec->mFrecency = NetworkEndian::readUint32(ptr); ptr += sizeof(uint32_t);
+    mRec->mOriginAttrsHash = NetworkEndian::readUint64(ptr); ptr += sizeof(uint64_t);
+    mRec->mExpirationTime = NetworkEndian::readUint32(ptr); ptr += sizeof(uint32_t);
+    mRec->mFlags = NetworkEndian::readUint32(ptr);
   }
 
   void Log() const {
     LOG(("CacheIndexEntry::Log() [this=%p, hash=%08x%08x%08x%08x%08x, fresh=%u,"
          " initialized=%u, removed=%u, dirty=%u, anonymous=%u, "
          "originAttrsHash=%llx, frecency=%u, expirationTime=%u, size=%u]",
          this, LOGSHA1(mRec->mHash), IsFresh(), IsInitialized(), IsRemoved(),
          IsDirty(), Anonymous(), OriginAttrsHash(), GetFrecency(),