From fd0a0d2b7ae9dd3d8a02b6a12e7941f7189fbb6c Mon Sep 17 00:00:00 2001 From: Yi Wang Date: Fri, 27 Oct 2017 10:46:15 -0700 Subject: [PATCH] Create LongStringDictionary and replace SimpleStringDictionary usages in client/ios/Breakpad.mm. Bug: Change-Id: I401028f5d90417d79fb109b510aaa9660a039b44 Reviewed-on: https://chromium-review.googlesource.com/688301 Reviewed-by: Mark Mentovai --- src/client/ios/Breakpad.mm | 40 +-- .../ios/Breakpad.xcodeproj/project.pbxproj | 8 + src/common/common.gyp | 3 + src/common/long_string_dictionary.cc | 175 ++++++++++ src/common/long_string_dictionary.h | 87 +++++ src/common/long_string_dictionary_unittest.cc | 301 ++++++++++++++++++ src/common/simple_string_dictionary.h | 6 +- 7 files changed, 599 insertions(+), 21 deletions(-) create mode 100644 src/common/long_string_dictionary.cc create mode 100644 src/common/long_string_dictionary.h create mode 100644 src/common/long_string_dictionary_unittest.cc diff --git a/src/client/ios/Breakpad.mm b/src/client/ios/Breakpad.mm index 88dd2870..ee36a04e 100644 --- a/src/client/ios/Breakpad.mm +++ b/src/client/ios/Breakpad.mm @@ -38,13 +38,15 @@ #include #include +#include + #import "client/ios/handler/ios_exception_minidump_generator.h" #import "client/mac/crash_generation/ConfigFile.h" #import "client/mac/handler/exception_handler.h" #import "client/mac/handler/minidump_generator.h" -#import "client/mac/sender/uploader.h" #import "client/mac/handler/protected_memory_allocator.h" -#import "common/simple_string_dictionary.h" +#import "client/mac/sender/uploader.h" +#import "common/long_string_dictionary.h" #if !TARGET_OS_TV && !TARGET_OS_WATCH #import "client/mac/handler/exception_handler.h" @@ -66,7 +68,7 @@ using google_breakpad::ConfigFile; using google_breakpad::EnsureDirectoryPathExists; -using google_breakpad::SimpleStringDictionary; +using google_breakpad::LongStringDictionary; //============================================================================= // We want any memory allocations which are used by breakpad during the @@ -197,7 +199,7 @@ class Breakpad { // MachineExceptions.h, we have to explicitly name the handler. google_breakpad::ExceptionHandler *handler_; // The actual handler (STRONG) - SimpleStringDictionary *config_params_; // Create parameters (STRONG) + LongStringDictionary *config_params_; // Create parameters (STRONG) ConfigFile config_file_; @@ -313,7 +315,7 @@ Breakpad::~Breakpad() { // since they were allocated by ProtectedMemoryAllocator objects. // if (config_params_) { - config_params_->~SimpleStringDictionary(); + config_params_->~LongStringDictionary(); } if (handler_) @@ -381,10 +383,10 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) { } config_params_ = - new (gKeyValueAllocator->Allocate(sizeof(SimpleStringDictionary)) ) - SimpleStringDictionary(); + new (gKeyValueAllocator->Allocate(sizeof(LongStringDictionary))) + LongStringDictionary(); - SimpleStringDictionary &dictionary = *config_params_; + LongStringDictionary &dictionary = *config_params_; dictionary.SetKeyValue(BREAKPAD_SERVER_TYPE, [serverType UTF8String]); dictionary.SetKeyValue(BREAKPAD_PRODUCT_DISPLAY, [display UTF8String]); @@ -427,8 +429,8 @@ NSString *Breakpad::KeyValue(NSString *key) { if (!config_params_ || !key) return nil; - const char *value = config_params_->GetValueForKey([key UTF8String]); - return value ? [NSString stringWithUTF8String:value] : nil; + const std::string value = config_params_->GetValueForKey([key UTF8String]); + return value.empty() ? nil : [NSString stringWithUTF8String:value.c_str()]; } //============================================================================= @@ -502,8 +504,8 @@ void Breakpad::UploadData(NSData *data, NSString *name, NSDictionary *server_parameters) { NSMutableDictionary *config = [NSMutableDictionary dictionary]; - SimpleStringDictionary::Iterator it(*config_params_); - while (const SimpleStringDictionary::Entry *next = it.Next()) { + LongStringDictionary::Iterator it(*config_params_); + while (const LongStringDictionary::Entry *next = it.Next()) { [config setValue:[NSString stringWithUTF8String:next->value] forKey:[NSString stringWithUTF8String:next->key]]; } @@ -532,7 +534,7 @@ NSDictionary *Breakpad::GenerateReport(NSDictionary *server_parameters) { if (!success) return nil; - SimpleStringDictionary params = *config_params_; + LongStringDictionary params = *config_params_; for (NSString *key in server_parameters) { params.SetKeyValue([key UTF8String], [[server_parameters objectForKey:key] UTF8String]); @@ -567,7 +569,7 @@ bool Breakpad::HandleMinidump(const char *dump_dir, void Breakpad::HandleUncaughtException(NSException *exception) { // Generate the minidump. google_breakpad::IosExceptionMinidumpGenerator generator(exception); - const char *minidump_path = + const std::string minidump_path = config_params_->GetValueForKey(BREAKPAD_DUMP_DIRECTORY); std::string minidump_id; std::string minidump_filename = generator.UniqueNameInDirectory(minidump_path, @@ -580,7 +582,7 @@ void Breakpad::HandleUncaughtException(NSException *exception) { // 2- If the application crash while trying to handle this exception, a usual // report will be generated. This report must not contain these special // keys. - SimpleStringDictionary params = *config_params_; + LongStringDictionary params = *config_params_; params.SetKeyValue(BREAKPAD_SERVER_PARAMETER_PREFIX "type", "exception"); params.SetKeyValue(BREAKPAD_SERVER_PARAMETER_PREFIX "exceptionName", [[exception name] UTF8String]); @@ -589,9 +591,9 @@ void Breakpad::HandleUncaughtException(NSException *exception) { // And finally write the config file. ConfigFile config_file; - config_file.WriteFile(minidump_path, + config_file.WriteFile(minidump_path.c_str(), ¶ms, - minidump_path, + minidump_path.c_str(), minidump_id.c_str()); } @@ -619,9 +621,9 @@ BreakpadRef BreakpadCreate(NSDictionary *parameters) { gKeyValueAllocator = new (gMasterAllocator->Allocate(sizeof(ProtectedMemoryAllocator))) - ProtectedMemoryAllocator(sizeof(SimpleStringDictionary)); + ProtectedMemoryAllocator(sizeof(LongStringDictionary)); - // Create a mutex for use in accessing the SimpleStringDictionary + // Create a mutex for use in accessing the LongStringDictionary int mutexResult = pthread_mutex_init(&gDictionaryMutex, NULL); if (mutexResult == 0) { diff --git a/src/client/ios/Breakpad.xcodeproj/project.pbxproj b/src/client/ios/Breakpad.xcodeproj/project.pbxproj index e9fcae3f..e047b51b 100644 --- a/src/client/ios/Breakpad.xcodeproj/project.pbxproj +++ b/src/client/ios/Breakpad.xcodeproj/project.pbxproj @@ -57,6 +57,8 @@ 1EEEB6101720821900F7E689 /* simple_string_dictionary.h in Headers */ = {isa = PBXBuildFile; fileRef = 1EEEB60D1720821900F7E689 /* simple_string_dictionary.h */; }; AA747D9F0F9514B9006C5449 /* Breakpad_Prefix.pch in Headers */ = {isa = PBXBuildFile; fileRef = AA747D9E0F9514B9006C5449 /* Breakpad_Prefix.pch */; }; AACBBE4A0F95108600F1A2B1 /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AACBBE490F95108600F1A2B1 /* Foundation.framework */; }; + CF6D547D1F9E6FFE00E95174 /* long_string_dictionary.cc in Sources */ = {isa = PBXBuildFile; fileRef = CF6D547C1F9E6FFE00E95174 /* long_string_dictionary.cc */; }; + CF706DC11F7C6EFB002C54C7 /* long_string_dictionary.h in Headers */ = {isa = PBXBuildFile; fileRef = CF706DC01F7C6EFB002C54C7 /* long_string_dictionary.h */; }; /* End PBXBuildFile section */ /* Begin PBXFileReference section */ @@ -111,6 +113,8 @@ 1EEEB60D1720821900F7E689 /* simple_string_dictionary.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = simple_string_dictionary.h; sourceTree = ""; }; AA747D9E0F9514B9006C5449 /* Breakpad_Prefix.pch */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Breakpad_Prefix.pch; sourceTree = SOURCE_ROOT; }; AACBBE490F95108600F1A2B1 /* Foundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Foundation.framework; path = System/Library/Frameworks/Foundation.framework; sourceTree = SDKROOT; }; + CF6D547C1F9E6FFE00E95174 /* long_string_dictionary.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = long_string_dictionary.cc; sourceTree = ""; }; + CF706DC01F7C6EFB002C54C7 /* long_string_dictionary.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = long_string_dictionary.h; sourceTree = ""; }; D2AAC07E0554694100DB518D /* libBreakpad.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libBreakpad.a; sourceTree = BUILT_PRODUCTS_DIR; }; /* End PBXFileReference section */ @@ -264,6 +268,8 @@ 16C7CC47147D4A4300776EAD /* common */ = { isa = PBXGroup; children = ( + CF706DC01F7C6EFB002C54C7 /* long_string_dictionary.h */, + CF6D547C1F9E6FFE00E95174 /* long_string_dictionary.cc */, 1EEEB60C1720821900F7E689 /* simple_string_dictionary.cc */, 1EEEB60D1720821900F7E689 /* simple_string_dictionary.h */, 16C7CC4A147D4A4300776EAD /* convert_UTF.c */, @@ -339,6 +345,7 @@ 16C7CEA8147D4A4300776EAD /* string_conversion.h in Headers */, 16BFA67014E195E9009704F8 /* ios_exception_minidump_generator.h in Headers */, 16C92FAD150DF8330053D7BA /* BreakpadController.h in Headers */, + CF706DC11F7C6EFB002C54C7 /* long_string_dictionary.h in Headers */, 1EEEB6101720821900F7E689 /* simple_string_dictionary.h in Headers */, 14569323182CE2C10029C465 /* mach_vm_compat.h in Headers */, ); @@ -416,6 +423,7 @@ 16C7CDFC147D4A4300776EAD /* minidump_generator.cc in Sources */, 16C7CDFE147D4A4300776EAD /* protected_memory_allocator.cc in Sources */, 16C7CE09147D4A4300776EAD /* uploader.mm in Sources */, + CF6D547D1F9E6FFE00E95174 /* long_string_dictionary.cc in Sources */, 16C7CE19147D4A4300776EAD /* minidump_file_writer.cc in Sources */, 16C7CE40147D4A4300776EAD /* convert_UTF.c in Sources */, 16C7CE79147D4A4300776EAD /* GTMLogger.m in Sources */, diff --git a/src/common/common.gyp b/src/common/common.gyp index 9cc90f84..e2ea4007 100644 --- a/src/common/common.gyp +++ b/src/common/common.gyp @@ -121,6 +121,8 @@ 'linux/safe_readlink.h', 'linux/synth_elf.cc', 'linux/synth_elf.h', + 'long_string_dictionary.cc', + 'long_string_dictionary.h', 'mac/arch_utilities.cc', 'mac/arch_utilities.h', 'mac/bootstrap_compat.cc', @@ -220,6 +222,7 @@ 'linux/tests/auto_testfile.h', 'linux/tests/crash_generator.cc', 'linux/tests/crash_generator.h', + 'long_string_dictionary_unittest.cc', 'mac/macho_reader_unittest.cc', 'memory_allocator_unittest.cc', 'memory_range_unittest.cc', diff --git a/src/common/long_string_dictionary.cc b/src/common/long_string_dictionary.cc new file mode 100644 index 00000000..906ad39f --- /dev/null +++ b/src/common/long_string_dictionary.cc @@ -0,0 +1,175 @@ +// Copyright (c) 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include "common/long_string_dictionary.h" + +#include + +#include +#include + +#include "common/simple_string_dictionary.h" + +#define arraysize(f) (sizeof(f) / sizeof(*f)) + +namespace { + // Suffixes for segment keys. + const char* const kSuffixes[] = {"__1", "__2", "__3", "__4", "__5", "__6", + "__7", "__8", "__9", "__10"}; + // The maximum suffix string length. + const size_t kMaxSuffixLength = 4; +} // namespace + +namespace google_breakpad { + +using std::string; + +void LongStringDictionary::SetKeyValue(const char* key, const char* value) { + assert(key); + if (!key) + return; + + RemoveKey(key); + + if (!value) { + return; + } + + // Key must not be an empty string. + assert(key[0] != '\0'); + if (key[0] == '\0') + return; + + // If the value is not valid for segmentation, forwards the key and the value + // to SetKeyValue of SimpleStringDictionary and returns. + size_t value_length = strlen(value); + if (value_length <= (value_size - 1)) { + SimpleStringDictionary::SetKeyValue(key, value); + return; + } + + size_t key_length = strlen(key); + assert(key_length + kMaxSuffixLength <= (key_size - 1)); + + char segment_key[key_size]; + char segment_value[value_size]; + + strcpy(segment_key, key); + + const char* remain_value = value; + size_t remain_value_length = strlen(value); + + for (unsigned long i = 0; i < arraysize(kSuffixes); i++) { + if (remain_value_length == 0) { + return; + } + + strcpy(segment_key + key_length, kSuffixes[i]); + + size_t segment_value_length = + std::min(remain_value_length, value_size - 1); + + strncpy(segment_value, remain_value, segment_value_length); + segment_value[segment_value_length] = '\0'; + + remain_value += segment_value_length; + remain_value_length -= segment_value_length; + + SimpleStringDictionary::SetKeyValue(segment_key, segment_value); + } +} + +bool LongStringDictionary::RemoveKey(const char* key) { + assert(key); + if (!key) + return false; + + if (SimpleStringDictionary::RemoveKey(key)) { + return true; + } + + size_t key_length = strlen(key); + assert(key_length + kMaxSuffixLength <= (key_size - 1)); + + char segment_key[key_size]; + strcpy(segment_key, key); + + unsigned long i = 0; + for (; i < arraysize(kSuffixes); i++) { + strcpy(segment_key + key_length, kSuffixes[i]); + if (!SimpleStringDictionary::RemoveKey(segment_key)) { + break; + } + } + return i != 0; +} + +const string LongStringDictionary::GetValueForKey(const char* key) const { + assert(key); + if (!key) + return ""; + + // Key must not be an empty string. + assert(key[0] != '\0'); + if (key[0] == '\0') + return ""; + + const char* value = SimpleStringDictionary::GetValueForKey(key); + if (value) + return string(value); + + size_t key_length = strlen(key); + assert(key_length + kMaxSuffixLength <= (key_size - 1)); + + bool found_segment = false; + char segment_key[key_size]; + string return_value; + + strcpy(segment_key, key); + for (unsigned long i = 0; i < arraysize(kSuffixes); i++) { + strcpy(segment_key + key_length, kSuffixes[i]); + + const char* segment_value = + SimpleStringDictionary::GetValueForKey(segment_key); + + if (segment_value != NULL) { + found_segment = true; + return_value.append(segment_value); + } else { + break; + } + } + + if (found_segment) { + return return_value; + } + return ""; +} + +} // namespace google_breakpad diff --git a/src/common/long_string_dictionary.h b/src/common/long_string_dictionary.h new file mode 100644 index 00000000..68bf03de --- /dev/null +++ b/src/common/long_string_dictionary.h @@ -0,0 +1,87 @@ +// Copyright (c) 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#ifndef COMMON_LONG_STRING_DICTIONARY_H_ +#define COMMON_LONG_STRING_DICTIONARY_H_ + +#include + +#include "common/simple_string_dictionary.h" + +namespace google_breakpad { +// key_size is the maxium size that |key| can take in +// SimpleStringDictionary which is defined in simple_string_dictionary.h. +// +// value_size is the maxium size that |value| can take in +// SimpleStringDictionary which is defined in simple_string_dictionary.h. +// +// LongStringDictionary is a subclass of SimpleStringDictionary which supports +// longer values to be stored in the dictionary. The maximum length supported is +// (value_size - 1) * 10. +// +// For example, LongStringDictionary will store long value with key 'abc' into +// segment values with segment keys 'abc__1', 'abc__2', 'abc__3', ... +// +// Clients must avoid using the same suffixes as their key's suffix when +// LongStringDictionary is used. +class LongStringDictionary : public SimpleStringDictionary { + public: + // Stores |value| into |key|, or segment values into segment keys. The maxium + // number of segments is 10. If |value| can not be stored in 10 segments, it + // will be truncated. Replacing the existing value if |key| is already present + // and replacing the existing segment values if segment keys are already + // present. + // + // |key| must not be NULL. If the |value| need to be divided into segments, + // the lengh of |key| must be smaller enough so that lengths of segment keys + // which are key with suffixes are all samller than (key_size - 1). Currently, + // the max length of suffixes are 4. + // + // If |value| is NULL, the key and its corresponding segment keys are removed + // from the map. If there is no more space in the map, then the operation + // silently fails. + void SetKeyValue(const char* key, const char* value); + + // Given |key|, removes any associated value or associated segment values. + // |key| must not be NULL. If the key is not found, searchs its segment keys + // and removes corresponding segment values if found. + bool RemoveKey(const char* key); + + // Given |key|, returns its corresponding |value|. |key| must not be NULL. If + // the key is found, its corresponding |value| is returned. + // + // If no corresponding |value| is found, segment keys of the given |key| will + // be used to search for corresponding segment values. If segment values + // exist, assembled value from them is returned. If no segment value exists, + // NULL is returned. + const std::string GetValueForKey(const char* key) const; +}; +} // namespace google_breakpad + +#endif // COMMON_LONG_STRING_DICTIONARY_H_ diff --git a/src/common/long_string_dictionary_unittest.cc b/src/common/long_string_dictionary_unittest.cc new file mode 100644 index 00000000..f9b645ba --- /dev/null +++ b/src/common/long_string_dictionary_unittest.cc @@ -0,0 +1,301 @@ +// Copyright (c) 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include +#include + +#include "breakpad_googletest_includes.h" +#include "common/long_string_dictionary.h" + +namespace google_breakpad { + +using std::string; + +TEST(LongStringDictionary, LongStringDictionary) { + // Make a new dictionary + LongStringDictionary dict; + + // Set three distinct values on three keys + dict.SetKeyValue("key1", "value1"); + dict.SetKeyValue("key2", "value2"); + dict.SetKeyValue("key3", "value3"); + + EXPECT_EQ("value1", dict.GetValueForKey("key1")); + EXPECT_EQ("value2", dict.GetValueForKey("key2")); + EXPECT_EQ("value3", dict.GetValueForKey("key3")); + EXPECT_EQ(3u, dict.GetCount()); + // try an unknown key + EXPECT_EQ("", dict.GetValueForKey("key4")); + + // Remove a key + dict.RemoveKey("key3"); + + // Now make sure it's not there anymore + EXPECT_EQ("", dict.GetValueForKey("key3")); + + // Remove by setting value to NULL + dict.SetKeyValue("key2", NULL); + + // Now make sure it's not there anymore + EXPECT_EQ("", dict.GetValueForKey("key2")); +} + +// Add a bunch of values to the dictionary, remove some entries in the middle, +// and then add more. +TEST(LongStringDictionary, Iterator) { + LongStringDictionary* dict = new LongStringDictionary(); + ASSERT_TRUE(dict); + + char key[LongStringDictionary::key_size]; + char value[LongStringDictionary::value_size]; + + const int kDictionaryCapacity = LongStringDictionary::num_entries; + const int kPartitionIndex = kDictionaryCapacity - 5; + + // We assume at least this size in the tests below + ASSERT_GE(kDictionaryCapacity, 64); + + // We'll keep track of the number of key/value pairs we think should + // be in the dictionary + int expectedDictionarySize = 0; + + // Set a bunch of key/value pairs like key0/value0, key1/value1, ... + for (int i = 0; i < kPartitionIndex; ++i) { + sprintf(key, "key%d", i); + sprintf(value, "value%d", i); + dict->SetKeyValue(key, value); + } + expectedDictionarySize = kPartitionIndex; + + // set a couple of the keys twice (with the same value) - should be nop + dict->SetKeyValue("key2", "value2"); + dict->SetKeyValue("key4", "value4"); + dict->SetKeyValue("key15", "value15"); + + // Remove some random elements in the middle + dict->RemoveKey("key7"); + dict->RemoveKey("key18"); + dict->RemoveKey("key23"); + dict->RemoveKey("key31"); + expectedDictionarySize -= 4; // we just removed four key/value pairs + + // Set some more key/value pairs like key59/value59, key60/value60, ... + for (int i = kPartitionIndex; i < kDictionaryCapacity; ++i) { + sprintf(key, "key%d", i); + sprintf(value, "value%d", i); + dict->SetKeyValue(key, value); + } + expectedDictionarySize += kDictionaryCapacity - kPartitionIndex; + + // Now create an iterator on the dictionary + SimpleStringDictionary::Iterator iter(*dict); + + // We then verify that it iterates through exactly the number of + // key/value pairs we expect, and that they match one-for-one with what we + // would expect. The ordering of the iteration does not matter... + + // used to keep track of number of occurrences found for key/value pairs + int count[kDictionaryCapacity]; + memset(count, 0, sizeof(count)); + + int totalCount = 0; + + const SimpleStringDictionary::Entry* entry; + while ((entry = iter.Next())) { + totalCount++; + + // Extract keyNumber from a string of the form key + int keyNumber; + sscanf(entry->key, "key%d", &keyNumber); + + // Extract valueNumber from a string of the form value + int valueNumber; + sscanf(entry->value, "value%d", &valueNumber); + + // The value number should equal the key number since that's how we set them + EXPECT_EQ(keyNumber, valueNumber); + + // Key and value numbers should be in proper range: + // 0 <= keyNumber < kDictionaryCapacity + bool isKeyInGoodRange = (keyNumber >= 0 && keyNumber < kDictionaryCapacity); + bool isValueInGoodRange = + (valueNumber >= 0 && valueNumber < kDictionaryCapacity); + EXPECT_TRUE(isKeyInGoodRange); + EXPECT_TRUE(isValueInGoodRange); + + if (isKeyInGoodRange && isValueInGoodRange) { + ++count[keyNumber]; + } + } + + // Make sure each of the key/value pairs showed up exactly one time, except + // for the ones which we removed. + for (size_t i = 0; i < kDictionaryCapacity; ++i) { + // Skip over key7, key18, key23, and key31, since we removed them + if (!(i == 7 || i == 18 || i == 23 || i == 31)) { + EXPECT_EQ(count[i], 1); + } + } + + // Make sure the number of iterations matches the expected dictionary size. + EXPECT_EQ(totalCount, expectedDictionarySize); +} + +TEST(LongStringDictionary, AddRemove) { + LongStringDictionary dict; + dict.SetKeyValue("rob", "ert"); + dict.SetKeyValue("mike", "pink"); + dict.SetKeyValue("mark", "allays"); + + EXPECT_EQ(3u, dict.GetCount()); + EXPECT_EQ("ert", dict.GetValueForKey("rob")); + EXPECT_EQ("pink", dict.GetValueForKey("mike")); + EXPECT_EQ("allays", dict.GetValueForKey("mark")); + + dict.RemoveKey("mike"); + + EXPECT_EQ(2u, dict.GetCount()); + EXPECT_EQ("", dict.GetValueForKey("mike")); + + dict.SetKeyValue("mark", "mal"); + EXPECT_EQ(2u, dict.GetCount()); + EXPECT_EQ("mal", dict.GetValueForKey("mark")); + + dict.RemoveKey("mark"); + EXPECT_EQ(1u, dict.GetCount()); + EXPECT_EQ("", dict.GetValueForKey("mark")); +} + +TEST(LongStringDictionary, AddRemoveLongValue) { + LongStringDictionary dict; + + string long_value = string(256, 'x'); + dict.SetKeyValue("rob", long_value.c_str()); + + EXPECT_EQ(2u, dict.GetCount()); + + string long_value_part_1 = string(255, 'x'); + + EXPECT_EQ(long_value_part_1, dict.GetValueForKey("rob__1")); + EXPECT_EQ("x", dict.GetValueForKey("rob__2")); + + EXPECT_EQ(long_value, dict.GetValueForKey("rob")); + + dict.RemoveKey("rob"); + EXPECT_EQ(0u, dict.GetCount()); +} + +TEST(LongStringDictionary, AddRemoveSuperLongValue) { + LongStringDictionary dict; + + string long_value = string(255 * 10, 'x'); + dict.SetKeyValue("rob", long_value.c_str()); + + EXPECT_EQ(10u, dict.GetCount()); + + string long_value_part = string(255, 'x'); + + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__1")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__2")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__3")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__4")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__5")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__6")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__7")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__8")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__9")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__10")); + EXPECT_EQ(10u, dict.GetCount()); + + EXPECT_EQ(long_value, dict.GetValueForKey("rob")); + + dict.RemoveKey("rob"); + EXPECT_EQ(0u, dict.GetCount()); +} + +TEST(LongStringDictionary, TruncateSuperLongValue) { + LongStringDictionary dict; + + string long_value = string(255 * 11, 'x'); + dict.SetKeyValue("rob", long_value.c_str()); + + EXPECT_EQ(10u, dict.GetCount()); + + string long_value_part = string(255, 'x'); + + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__1")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__2")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__3")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__4")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__5")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__6")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__7")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__8")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__9")); + EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__10")); + EXPECT_EQ(10u, dict.GetCount()); + + string expected_long_value = string(255 * 10, 'x'); + EXPECT_EQ(expected_long_value, dict.GetValueForKey("rob")); + + dict.RemoveKey("rob"); + EXPECT_EQ(0u, dict.GetCount()); +} + +TEST(LongStringDictionary, OverrideLongValue) { + LongStringDictionary dict; + + string long_value = string(255 * 10, 'x'); + dict.SetKeyValue("rob", long_value.c_str()); + + EXPECT_EQ(10u, dict.GetCount()); + EXPECT_EQ(long_value, dict.GetValueForKey("rob")); + + dict.SetKeyValue("rob", "short_value"); + + EXPECT_EQ(1u, dict.GetCount()); + EXPECT_EQ("short_value", dict.GetValueForKey("rob")); +} + +TEST(LongStringDictionary, OverrideShortValue) { + LongStringDictionary dict; + + dict.SetKeyValue("rob", "short_value"); + + EXPECT_EQ(1u, dict.GetCount()); + EXPECT_EQ("short_value", dict.GetValueForKey("rob")); + + string long_value = string(255 * 10, 'x'); + dict.SetKeyValue("rob", long_value.c_str()); + + EXPECT_EQ(10u, dict.GetCount()); + EXPECT_EQ(long_value, dict.GetValueForKey("rob")); +} + +} // namespace google_breakpad diff --git a/src/common/simple_string_dictionary.h b/src/common/simple_string_dictionary.h index d2ab17fd..28c4bf1c 100644 --- a/src/common/simple_string_dictionary.h +++ b/src/common/simple_string_dictionary.h @@ -209,20 +209,22 @@ class NonAllocatingMap { // Given |key|, removes any associated value. |key| must not be NULL. If // the key is not found, this is a noop. - void RemoveKey(const char* key) { + bool RemoveKey(const char* key) { assert(key); if (!key) - return; + return false; Entry* entry = GetEntryForKey(key); if (entry) { entry->key[0] = '\0'; entry->value[0] = '\0'; + return true; } #ifndef NDEBUG assert(GetEntryForKey(key) == NULL); #endif + return false; } // Places a serialized version of the map into |map| and returns the size.