Remove alignment code from cache coders
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Nov 2014 17:32:04 +0000 (17:32 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Nov 2014 17:32:04 +0000 (17:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138927

Reviewed by Anders Carlsson.

Alignment code doesn't do anything useful, wastes space and produces wrong results
if the encoder and decoder buffers don't have the same 8-byte alignment.

* NetworkProcess/cache/NetworkCacheDecoder.cpp:
(WebKit::NetworkCacheDecoder::NetworkCacheDecoder):
(WebKit::NetworkCacheDecoder::bufferIsLargeEnoughToContain):
(WebKit::NetworkCacheDecoder::decodeFixedLengthData):
(WebKit::NetworkCacheDecoder::decodeNumber):
(WebKit::roundUpToAlignment): Deleted.
(WebKit::alignedBufferIsLargeEnoughToContain): Deleted.
(WebKit::NetworkCacheDecoder::alignBufferPosition): Deleted.
* NetworkProcess/cache/NetworkCacheDecoder.h:
(WebKit::NetworkCacheDecoder::currentOffset):
(WebKit::NetworkCacheDecoder::isInvalid):
(WebKit::NetworkCacheDecoder::markInvalid):
(WebKit::NetworkCacheDecoder::bufferIsLargeEnoughToContain):
* NetworkProcess/cache/NetworkCacheEncoder.cpp:
(WebKit::NetworkCacheEncoder::grow):
(WebKit::NetworkCacheEncoder::encodeFixedLengthData):
(WebKit::NetworkCacheEncoder::encodeNumber):
(WebKit::roundUpToAlignment): Deleted.
* NetworkProcess/cache/NetworkCacheEncoder.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@176398 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheEncoder.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheEncoder.h

index 2756abf..ccbf1ca 100644 (file)
@@ -1,3 +1,33 @@
+2014-11-20  Antti Koivisto  <antti@apple.com>
+
+        Remove alignment code from cache coders
+        https://bugs.webkit.org/show_bug.cgi?id=138927
+
+        Reviewed by Anders Carlsson.
+
+        Alignment code doesn't do anything useful, wastes space and produces wrong results
+        if the encoder and decoder buffers don't have the same 8-byte alignment.
+
+        * NetworkProcess/cache/NetworkCacheDecoder.cpp:
+        (WebKit::NetworkCacheDecoder::NetworkCacheDecoder):
+        (WebKit::NetworkCacheDecoder::bufferIsLargeEnoughToContain):
+        (WebKit::NetworkCacheDecoder::decodeFixedLengthData):
+        (WebKit::NetworkCacheDecoder::decodeNumber):
+        (WebKit::roundUpToAlignment): Deleted.
+        (WebKit::alignedBufferIsLargeEnoughToContain): Deleted.
+        (WebKit::NetworkCacheDecoder::alignBufferPosition): Deleted.
+        * NetworkProcess/cache/NetworkCacheDecoder.h:
+        (WebKit::NetworkCacheDecoder::currentOffset):
+        (WebKit::NetworkCacheDecoder::isInvalid):
+        (WebKit::NetworkCacheDecoder::markInvalid):
+        (WebKit::NetworkCacheDecoder::bufferIsLargeEnoughToContain):
+        * NetworkProcess/cache/NetworkCacheEncoder.cpp:
+        (WebKit::NetworkCacheEncoder::grow):
+        (WebKit::NetworkCacheEncoder::encodeFixedLengthData):
+        (WebKit::NetworkCacheEncoder::encodeNumber):
+        (WebKit::roundUpToAlignment): Deleted.
+        * NetworkProcess/cache/NetworkCacheEncoder.h:
+
 2014-11-20  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         [GTK] URTBF after r176363.
index 8cda0a4..a297905 100644 (file)
@@ -34,7 +34,7 @@ namespace WebKit {
 
 NetworkCacheDecoder::NetworkCacheDecoder(const uint8_t* buffer, size_t bufferSize)
     : m_buffer(buffer)
-    , m_bufferPos(buffer)
+    , m_bufferPosition(buffer)
     , m_bufferEnd(buffer + bufferSize)
     , m_checksum(0)
 {
@@ -44,45 +44,18 @@ NetworkCacheDecoder::~NetworkCacheDecoder()
 {
 }
 
-static inline uint8_t* roundUpToAlignment(const uint8_t* ptr, unsigned alignment)
+bool NetworkCacheDecoder::bufferIsLargeEnoughToContain(size_t size) const
 {
-    // Assert that the alignment is a power of 2.
-    ASSERT(alignment && !(alignment & (alignment - 1)));
-
-    uintptr_t alignmentMask = alignment - 1;
-    return reinterpret_cast<uint8_t*>((reinterpret_cast<uintptr_t>(ptr) + alignmentMask) & ~alignmentMask);
-}
-
-static inline bool alignedBufferIsLargeEnoughToContain(const uint8_t* alignedPosition, const uint8_t* bufferEnd, size_t size)
-{
-    return bufferEnd >= alignedPosition && static_cast<size_t>(bufferEnd - alignedPosition) >= size;
-}
-
-bool NetworkCacheDecoder::alignBufferPosition(unsigned alignment, size_t size)
-{
-    uint8_t* alignedPosition = roundUpToAlignment(m_bufferPos, alignment);
-    if (!alignedBufferIsLargeEnoughToContain(alignedPosition, m_bufferEnd, size)) {
-        // We've walked off the end of this buffer.
-        markInvalid();
-        return false;
-    }
-    
-    m_bufferPos = alignedPosition;
-    return true;
-}
-
-bool NetworkCacheDecoder::bufferIsLargeEnoughToContain(unsigned alignment, size_t size) const
-{
-    return alignedBufferIsLargeEnoughToContain(roundUpToAlignment(m_bufferPos, alignment), m_bufferEnd, size);
+    return m_bufferPosition + size <= m_bufferEnd;
 }
 
-bool NetworkCacheDecoder::decodeFixedLengthData(uint8_t* data, size_t size, unsigned alignment)
+bool NetworkCacheDecoder::decodeFixedLengthData(uint8_t* data, size_t size)
 {
-    if (!alignBufferPosition(alignment, size))
+    if (!bufferIsLargeEnoughToContain(size))
         return false;
 
-    memcpy(data, m_bufferPos, size);
-    m_bufferPos += size;
+    memcpy(data, m_bufferPosition, size);
+    m_bufferPosition += size;
 
     NetworkCacheEncoder::updateChecksumForData(m_checksum, data, size);
     return true;
@@ -91,11 +64,11 @@ bool NetworkCacheDecoder::decodeFixedLengthData(uint8_t* data, size_t size, unsi
 template<typename Type>
 bool NetworkCacheDecoder::decodeNumber(Type& value)
 {
-    if (!alignBufferPosition(sizeof(value), sizeof(value)))
+    if (!bufferIsLargeEnoughToContain(sizeof(value)))
         return false;
 
-    memcpy(&value, m_bufferPos, sizeof(value));
-    m_bufferPos += sizeof(Type);
+    memcpy(&value, m_bufferPosition, sizeof(value));
+    m_bufferPosition += sizeof(Type);
 
     NetworkCacheEncoder::updateChecksumForNumber(m_checksum, value);
     return true;
index dbe04a6..0d2e970 100644 (file)
@@ -39,14 +39,14 @@ public:
     virtual ~NetworkCacheDecoder();
 
     size_t length() const { return m_bufferEnd - m_buffer; }
-    size_t currentOffset() const { return m_bufferPos - m_buffer; }
+    size_t currentOffset() const { return m_bufferPosition - m_buffer; }
 
-    bool isInvalid() const { return m_bufferPos > m_bufferEnd; }
-    void markInvalid() { m_bufferPos = m_bufferEnd + 1; }
+    bool isInvalid() const { return m_bufferPosition > m_bufferEnd; }
+    void markInvalid() { m_bufferPosition = m_bufferEnd + 1; }
 
     bool verifyChecksum();
 
-    bool decodeFixedLengthData(uint8_t*, size_t, unsigned alignment);
+    bool decodeFixedLengthData(uint8_t*, size_t);
 
     bool decode(bool&);
     bool decode(uint8_t&);
@@ -83,16 +83,15 @@ public:
         if (numElements > std::numeric_limits<size_t>::max() / sizeof(T))
             return false;
 
-        return bufferIsLargeEnoughToContain(alignof(T), numElements * sizeof(T));
+        return bufferIsLargeEnoughToContain(numElements * sizeof(T));
     }
 
 private:
-    bool alignBufferPosition(unsigned alignment, size_t);
-    bool bufferIsLargeEnoughToContain(unsigned alignment, size_t) const;
+    bool bufferIsLargeEnoughToContain(size_t) const;
     template<typename Type> bool decodeNumber(Type&);
 
     const uint8_t* m_buffer;
-    const uint8_t* m_bufferPos;
+    const uint8_t* m_bufferPosition;
     const uint8_t* m_bufferEnd;
 
     unsigned m_checksum;
index 90aad67..6b5ac16 100644 (file)
@@ -39,18 +39,11 @@ NetworkCacheEncoder::~NetworkCacheEncoder()
 {
 }
 
-static inline size_t roundUpToAlignment(size_t value, unsigned alignment)
+uint8_t* NetworkCacheEncoder::grow(size_t size)
 {
-    return ((value + alignment - 1) / alignment) * alignment;
-}
-
-uint8_t* NetworkCacheEncoder::grow(unsigned alignment, size_t size)
-{
-    size_t alignedSize = roundUpToAlignment(m_buffer.size(), alignment);
-
-    m_buffer.grow(alignedSize + size);
-    
-    return m_buffer.data() + alignedSize;
+    uint8_t* position = m_buffer.data() + m_buffer.size();
+    m_buffer.grow(m_buffer.size() + size);
+    return position;
 }
 
 void NetworkCacheEncoder::updateChecksumForData(unsigned& checksum, const uint8_t* data, size_t size)
@@ -61,23 +54,20 @@ void NetworkCacheEncoder::updateChecksumForData(unsigned& checksum, const uint8_
     checksum = WTF::pairIntHash(checksum, hash);
 }
 
-void NetworkCacheEncoder::encodeFixedLengthData(const uint8_t* data, size_t size, unsigned alignment)
+void NetworkCacheEncoder::encodeFixedLengthData(const uint8_t* data, size_t size)
 {
-    ASSERT(!(reinterpret_cast<uintptr_t>(data) % alignment));
-
     updateChecksumForData(m_checksum, data, size);
 
-    uint8_t* buffer = grow(alignment, size);
+    uint8_t* buffer = grow(size);
     memcpy(buffer, data, size);
 }
 
 template<typename Type>
 void NetworkCacheEncoder::encodeNumber(Type value)
 {
-    uint8_t* buffer = grow(sizeof(Type), sizeof(Type));
-
     NetworkCacheEncoder::updateChecksumForNumber(m_checksum, value);
 
+    uint8_t* buffer = grow(sizeof(Type));
     memcpy(buffer, &value, sizeof(Type));
 }
 
index a3229b4..d6a4ae0 100644 (file)
@@ -45,7 +45,7 @@ public:
     virtual ~NetworkCacheEncoder();
 
     void encodeChecksum();
-    void encodeFixedLengthData(const uint8_t*, size_t, unsigned alignment);
+    void encodeFixedLengthData(const uint8_t*, size_t);
 
     template<typename T> void encodeEnum(T t)
     {
@@ -84,7 +84,7 @@ private:
 
     template<typename Type> void encodeNumber(Type);
 
-    uint8_t* grow(unsigned alignment, size_t);
+    uint8_t* grow(size_t);
 
     template <typename Type> struct Salt;