diff options
author | pkasting <pkasting@chromium.org> | 2014-10-01 20:01:04 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-02 03:01:21 +0000 |
commit | 89a19f1430afa495acfccbc3ed7da9f51a7911d1 (patch) | |
tree | c0034324912fd6e1057cbd57b7cacb6555f72a95 /base | |
parent | 413cdda9719e7e8b4e9410a9b9630e684e29fc0c (diff) | |
download | chromium_src-89a19f1430afa495acfccbc3ed7da9f51a7911d1.zip chromium_src-89a19f1430afa495acfccbc3ed7da9f51a7911d1.tar.gz chromium_src-89a19f1430afa495acfccbc3ed7da9f51a7911d1.tar.bz2 |
Add Read/WriteSizeT() functions to the pickle layer, plus one consumer.
This eliminates the need for callers to do explicit conversions, and also
ensures callers don't try to implement pickling of a size_t using a 32-bit type,
leading to truncation on 64-bit targets. The pickle layer will ensure 64-bit
types are always used.
I'll be changing other callsites to use this in future patches.
BUG=none
TEST=none
Review URL: https://codereview.chromium.org/601563003
Cr-Commit-Position: refs/heads/master@{#297774}
Diffstat (limited to 'base')
-rw-r--r-- | base/metrics/histogram.cc | 14 | ||||
-rw-r--r-- | base/pickle.cc | 10 | ||||
-rw-r--r-- | base/pickle.h | 10 | ||||
-rw-r--r-- | base/pickle_unittest.cc | 99 |
4 files changed, 106 insertions, 27 deletions
diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc index 6bab0eb..0a4fcc0 100644 --- a/base/metrics/histogram.cc +++ b/base/metrics/histogram.cc @@ -37,13 +37,13 @@ bool ReadHistogramArguments(PickleIterator* iter, int* flags, int* declared_min, int* declared_max, - uint64* bucket_count, + size_t* bucket_count, uint32* range_checksum) { if (!iter->ReadString(histogram_name) || !iter->ReadInt(flags) || !iter->ReadInt(declared_min) || !iter->ReadInt(declared_max) || - !iter->ReadUInt64(bucket_count) || + !iter->ReadSizeT(bucket_count) || !iter->ReadUInt32(range_checksum)) { DLOG(ERROR) << "Pickle error decoding Histogram: " << *histogram_name; return false; @@ -297,7 +297,7 @@ bool Histogram::SerializeInfoImpl(Pickle* pickle) const { pickle->WriteInt(flags()) && pickle->WriteInt(declared_min()) && pickle->WriteInt(declared_max()) && - pickle->WriteUInt64(bucket_count()) && + pickle->WriteSizeT(bucket_count()) && pickle->WriteUInt32(bucket_ranges()->checksum()); } @@ -347,7 +347,7 @@ HistogramBase* Histogram::DeserializeInfoImpl(PickleIterator* iter) { int flags; int declared_min; int declared_max; - uint64 bucket_count; + size_t bucket_count; uint32 range_checksum; if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min, @@ -636,7 +636,7 @@ HistogramBase* LinearHistogram::DeserializeInfoImpl(PickleIterator* iter) { int flags; int declared_min; int declared_max; - uint64 bucket_count; + size_t bucket_count; uint32 range_checksum; if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min, @@ -691,7 +691,7 @@ HistogramBase* BooleanHistogram::DeserializeInfoImpl(PickleIterator* iter) { int flags; int declared_min; int declared_max; - uint64 bucket_count; + size_t bucket_count; uint32 range_checksum; if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min, @@ -786,7 +786,7 @@ HistogramBase* CustomHistogram::DeserializeInfoImpl(PickleIterator* iter) { int flags; int declared_min; int declared_max; - uint64 bucket_count; + size_t bucket_count; uint32 range_checksum; if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min, diff --git a/base/pickle.cc b/base/pickle.cc index 2d7a051..d461f41 100644 --- a/base/pickle.cc +++ b/base/pickle.cc @@ -106,6 +106,16 @@ bool PickleIterator::ReadUInt64(uint64* result) { return ReadBuiltinType(result); } +bool PickleIterator::ReadSizeT(size_t* result) { + // Always read size_t as a 64-bit value to ensure compatibility between 32-bit + // and 64-bit processes. + uint64 result_uint64 = 0; + bool success = ReadBuiltinType(&result_uint64); + *result = static_cast<size_t>(result_uint64); + // Fail if the cast above truncates the value. + return success && (*result == result_uint64); +} + bool PickleIterator::ReadFloat(float* result) { // crbug.com/315213 // The source data may not be properly aligned, and unaligned float reads diff --git a/base/pickle.h b/base/pickle.h index 99add5e..11cf484 100644 --- a/base/pickle.h +++ b/base/pickle.h @@ -35,6 +35,7 @@ class BASE_EXPORT PickleIterator { bool ReadUInt32(uint32* result) WARN_UNUSED_RESULT; bool ReadInt64(int64* result) WARN_UNUSED_RESULT; bool ReadUInt64(uint64* result) WARN_UNUSED_RESULT; + bool ReadSizeT(size_t* result) WARN_UNUSED_RESULT; bool ReadFloat(float* result) WARN_UNUSED_RESULT; bool ReadDouble(double* result) WARN_UNUSED_RESULT; bool ReadString(std::string* result) WARN_UNUSED_RESULT; @@ -172,6 +173,10 @@ class BASE_EXPORT Pickle { uint64* result) const WARN_UNUSED_RESULT { return iter->ReadUInt64(result); } + bool ReadSizeT(PickleIterator* iter, + size_t* result) const WARN_UNUSED_RESULT { + return iter->ReadSizeT(result); + } bool ReadFloat(PickleIterator* iter, float* result) const WARN_UNUSED_RESULT { return iter->ReadFloat(result); @@ -248,6 +253,11 @@ class BASE_EXPORT Pickle { bool WriteUInt64(uint64 value) { return WritePOD(value); } + bool WriteSizeT(size_t value) { + // Always write size_t as a 64-bit value to ensure compatibility between + // 32-bit and 64-bit processes. + return WritePOD(static_cast<uint64>(value)); + } bool WriteFloat(float value) { return WritePOD(value); } diff --git a/base/pickle_unittest.cc b/base/pickle_unittest.cc index db529a4..20a8d67 100644 --- a/base/pickle_unittest.cc +++ b/base/pickle_unittest.cc @@ -8,6 +8,7 @@ #include "base/memory/scoped_ptr.h" #include "base/pickle.h" #include "base/strings/string16.h" +#include "base/strings/utf_string_conversions.h" #include "testing/gtest/include/gtest/gtest.h" // Remove when this file is in the base namespace. @@ -15,43 +16,61 @@ using base::string16; namespace { -const int testint = 2093847192; -const std::string teststr("Hello world"); // note non-aligned string length -const std::wstring testwstr(L"Hello, world"); -const char testdata[] = "AAA\0BBB\0"; -const int testdatalen = arraysize(testdata) - 1; const bool testbool1 = false; const bool testbool2 = true; +const int testint = 2093847192; +const long testlong = 1093847192; const uint16 testuint16 = 32123; +const uint32 testuint32 = 1593847192; +const int64 testint64 = -0x7E8CA9253104BDFCLL; +const uint64 testuint64 = 0xCE8CA9253104BDF7ULL; +const size_t testsizet = 0xFEDC7654; const float testfloat = 3.1415926935f; const double testdouble = 2.71828182845904523; +const std::string teststring("Hello world"); // note non-aligned string length +const std::wstring testwstring(L"Hello, world"); +const base::string16 teststring16(base::ASCIIToUTF16("Hello, world")); +const char testdata[] = "AAA\0BBB\0"; +const int testdatalen = arraysize(testdata) - 1; // checks that the result void VerifyResult(const Pickle& pickle) { PickleIterator iter(pickle); - int outint; - EXPECT_TRUE(pickle.ReadInt(&iter, &outint)); - EXPECT_EQ(testint, outint); - - std::string outstr; - EXPECT_TRUE(pickle.ReadString(&iter, &outstr)); - EXPECT_EQ(teststr, outstr); - - std::wstring outwstr; - EXPECT_TRUE(pickle.ReadWString(&iter, &outwstr)); - EXPECT_EQ(testwstr, outwstr); - bool outbool; EXPECT_TRUE(pickle.ReadBool(&iter, &outbool)); EXPECT_FALSE(outbool); EXPECT_TRUE(pickle.ReadBool(&iter, &outbool)); EXPECT_TRUE(outbool); + int outint; + EXPECT_TRUE(pickle.ReadInt(&iter, &outint)); + EXPECT_EQ(testint, outint); + + long outlong; + EXPECT_TRUE(pickle.ReadLong(&iter, &outlong)); + EXPECT_EQ(testlong, outlong); + uint16 outuint16; EXPECT_TRUE(pickle.ReadUInt16(&iter, &outuint16)); EXPECT_EQ(testuint16, outuint16); + uint32 outuint32; + EXPECT_TRUE(pickle.ReadUInt32(&iter, &outuint32)); + EXPECT_EQ(testuint32, outuint32); + + int64 outint64; + EXPECT_TRUE(pickle.ReadInt64(&iter, &outint64)); + EXPECT_EQ(testint64, outint64); + + uint64 outuint64; + EXPECT_TRUE(pickle.ReadUInt64(&iter, &outuint64)); + EXPECT_EQ(testuint64, outuint64); + + size_t outsizet; + EXPECT_TRUE(pickle.ReadSizeT(&iter, &outsizet)); + EXPECT_EQ(testsizet, outsizet); + float outfloat; EXPECT_TRUE(pickle.ReadFloat(&iter, &outfloat)); EXPECT_EQ(testfloat, outfloat); @@ -60,6 +79,18 @@ void VerifyResult(const Pickle& pickle) { EXPECT_TRUE(pickle.ReadDouble(&iter, &outdouble)); EXPECT_EQ(testdouble, outdouble); + std::string outstring; + EXPECT_TRUE(pickle.ReadString(&iter, &outstring)); + EXPECT_EQ(teststring, outstring); + + std::wstring outwstring; + EXPECT_TRUE(pickle.ReadWString(&iter, &outwstring)); + EXPECT_EQ(testwstring, outwstring); + + base::string16 outstring16; + EXPECT_TRUE(pickle.ReadString16(&iter, &outstring16)); + EXPECT_EQ(teststring16, outstring16); + const char* outdata; int outdatalen; EXPECT_TRUE(pickle.ReadData(&iter, &outdata, &outdatalen)); @@ -75,14 +106,21 @@ void VerifyResult(const Pickle& pickle) { TEST(PickleTest, EncodeDecode) { Pickle pickle; - EXPECT_TRUE(pickle.WriteInt(testint)); - EXPECT_TRUE(pickle.WriteString(teststr)); - EXPECT_TRUE(pickle.WriteWString(testwstr)); EXPECT_TRUE(pickle.WriteBool(testbool1)); EXPECT_TRUE(pickle.WriteBool(testbool2)); + EXPECT_TRUE(pickle.WriteInt(testint)); + EXPECT_TRUE( + pickle.WriteLongUsingDangerousNonPortableLessPersistableForm(testlong)); EXPECT_TRUE(pickle.WriteUInt16(testuint16)); + EXPECT_TRUE(pickle.WriteUInt32(testuint32)); + EXPECT_TRUE(pickle.WriteInt64(testint64)); + EXPECT_TRUE(pickle.WriteUInt64(testuint64)); + EXPECT_TRUE(pickle.WriteSizeT(testsizet)); EXPECT_TRUE(pickle.WriteFloat(testfloat)); EXPECT_TRUE(pickle.WriteDouble(testdouble)); + EXPECT_TRUE(pickle.WriteString(teststring)); + EXPECT_TRUE(pickle.WriteWString(testwstring)); + EXPECT_TRUE(pickle.WriteString16(teststring16)); EXPECT_TRUE(pickle.WriteData(testdata, testdatalen)); VerifyResult(pickle); @@ -96,6 +134,27 @@ TEST(PickleTest, EncodeDecode) { VerifyResult(pickle3); } +// Tests that reading/writing a size_t works correctly when the source process +// is 64-bit. We rely on having both 32- and 64-bit trybots to validate both +// arms of the conditional in this test. +TEST(PickleTest, SizeTFrom64Bit) { + Pickle pickle; + // Under the hood size_t is always written as a 64-bit value, so simulate a + // 64-bit size_t even on 32-bit architectures by explicitly writing a uint64. + EXPECT_TRUE(pickle.WriteUInt64(testuint64)); + + PickleIterator iter(pickle); + size_t outsizet; + if (sizeof(size_t) < sizeof(uint64)) { + // ReadSizeT() should return false when the original written value can't be + // represented as a size_t. + EXPECT_FALSE(pickle.ReadSizeT(&iter, &outsizet)); + } else { + EXPECT_TRUE(pickle.ReadSizeT(&iter, &outsizet)); + EXPECT_EQ(testuint64, outsizet); + } +} + // Tests that we can handle really small buffers. TEST(PickleTest, SmallBuffer) { scoped_ptr<char[]> buffer(new char[1]); |