From bbd8e82a7fa12d8c56882ec6a9d6d42bebd7983c Mon Sep 17 00:00:00 2001 From: "SiyangXie@gmail.com" Date: Wed, 3 Nov 2010 23:54:01 +0000 Subject: [PATCH] Make memory allocation/deallocation consistent: use new char[] instead of operator new() git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@724 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/processor/map_serializers-inl.h | 8 +++---- src/processor/map_serializers.h | 4 ++++ src/processor/map_serializers_unittest.cc | 2 +- src/processor/module_comparer.cc | 29 ++++++++++------------- src/processor/module_serializer.cc | 5 ++-- src/processor/module_serializer.h | 6 ++--- 6 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/processor/map_serializers-inl.h b/src/processor/map_serializers-inl.h index 760c52e3..d68e8b9f 100644 --- a/src/processor/map_serializers-inl.h +++ b/src/processor/map_serializers-inl.h @@ -102,8 +102,7 @@ char *StdMapSerializer::Serialize( // Compute size of memory to be allocated. unsigned int size_to_alloc = SizeOf(m); // Allocate memory. - char *serialized_data = - reinterpret_cast(operator new(size_to_alloc)); + char *serialized_data = new char[size_to_alloc]; if (!serialized_data) { BPLOG(INFO) << "StdMapSerializer memory allocation failed."; if (size) *size = 0; @@ -172,8 +171,7 @@ char *RangeMapSerializer::Serialize( // Compute size of memory to be allocated. unsigned int size_to_alloc = SizeOf(m); // Allocate memory. - char *serialized_data = - reinterpret_cast(operator new(size_to_alloc)); + char *serialized_data = new char[size_to_alloc]; if (!serialized_data) { BPLOG(INFO) << "RangeMapSerializer memory allocation failed."; if (size) *size = 0; @@ -252,7 +250,7 @@ char *ContainedRangeMapSerializer::Serialize( const ContainedRangeMap *m, unsigned int *size) const { unsigned int size_to_alloc = SizeOf(m); // Allocating memory. - char *serialized_data = reinterpret_cast(operator new(size_to_alloc)); + char *serialized_data = new char[size_to_alloc]; if (!serialized_data) { BPLOG(INFO) << "ContainedRangeMapSerializer memory allocation failed."; if (size) *size = 0; diff --git a/src/processor/map_serializers.h b/src/processor/map_serializers.h index 9c68e3f7..a0b9d3fd 100644 --- a/src/processor/map_serializers.h +++ b/src/processor/map_serializers.h @@ -65,6 +65,7 @@ class StdMapSerializer { // described in "StaticMap.h" comment. // Returns a pointer to the serialized data. If size != NULL, *size is set // to the size of serialized data, i.e., SizeOf(m). + // Caller has the ownership of memory allocated as "new char[]". char* Serialize(const std::map &m, unsigned int *size) const; private: @@ -92,6 +93,7 @@ class AddressMapSerializer { // Serializes an AddressMap object into a chunk of memory data. // Returns a pointer to the serialized data. If size != NULL, *size is set // to the size of serialized data, i.e., SizeOf(m). + // Caller has the ownership of memory allocated as "new char[]". char* Serialize(const AddressMap &m, unsigned int *size) const { return std_map_serializer_.Serialize(m.map_, size); } @@ -118,6 +120,7 @@ class RangeMapSerializer { // Serializes a RangeMap object into a chunk of memory data. // Returns a pointer to the serialized data. If size != NULL, *size is set // to the size of serialized data, i.e., SizeOf(m). + // Caller has the ownership of memory allocated as "new char[]". char* Serialize(const RangeMap &m, unsigned int *size) const; private: @@ -147,6 +150,7 @@ class ContainedRangeMapSerializer { // Serializes a ContainedRangeMap object into a chunk of memory data. // Returns a pointer to the serialized data. If size != NULL, *size is set // to the size of serialized data, i.e., SizeOf(m). + // Caller has the ownership of memory allocated as "new char[]". char* Serialize(const ContainedRangeMap *m, unsigned int *size) const; diff --git a/src/processor/map_serializers_unittest.cc b/src/processor/map_serializers_unittest.cc index b1e3f868..c20ea0dc 100644 --- a/src/processor/map_serializers_unittest.cc +++ b/src/processor/map_serializers_unittest.cc @@ -59,7 +59,7 @@ class TestStdMapSerializer : public ::testing::Test { } void TearDown() { - delete serialized_data_; + delete [] serialized_data_; } std::map std_map_; diff --git a/src/processor/module_comparer.cc b/src/processor/module_comparer.cc index 85605dc3..837d854b 100644 --- a/src/processor/module_comparer.cc +++ b/src/processor/module_comparer.cc @@ -53,33 +53,28 @@ namespace google_breakpad { bool ModuleComparer::Compare(const string &symbol_data) { - // Empty CodeModule with only a name "test": - BasicCodeModule code_module(0, 0, "test", "", "", "", ""); + scoped_ptr basic_module(new BasicModule("test_module")); + scoped_ptr fast_module(new FastModule("test_module")); - // Load BasicSourceLineResolver::Module. - BPLOG(INFO) << "Unserialized size = " << symbol_data.size() << " Bytes"; - basic_resolver_->LoadModuleUsingMapBuffer(&code_module, symbol_data); - BasicModule *old_module = dynamic_cast( - basic_resolver_->modules_->at("test")); + // Load symbol data into basic_module + scoped_array buffer(new char[symbol_data.size() + 1]); + strcpy(buffer.get(), symbol_data.c_str()); + ASSERT_TRUE(basic_module->LoadMapFromMemory(buffer.get())); + buffer.reset(); // Serialize BasicSourceLineResolver::Module. unsigned int serialized_size = 0; - char *mem = serializer_.Serialize(*old_module, &serialized_size); - ASSERT_TRUE(mem); + scoped_array serialized_data( + serializer_.Serialize(*(basic_module.get()), &serialized_size)); + ASSERT_TRUE(serialized_data.get()); BPLOG(INFO) << "Serialized size = " << serialized_size << " Bytes"; // Load FastSourceLineResolver::Module using serialized data. - ASSERT_TRUE(fast_resolver_->LoadModuleUsingMemoryBuffer(&code_module, mem)); - FastModule *new_module = dynamic_cast( - fast_resolver_->modules_->at("test")); + ASSERT_TRUE(fast_module->LoadMapFromMemory(serialized_data.get())); // Compare FastSourceLineResolver::Module with // BasicSourceLineResolver::Module. - ASSERT_TRUE(CompareModule(old_module, new_module)); - - // Clean up. - basic_resolver_->UnloadModule(&code_module); - fast_resolver_->UnloadModule(&code_module); + ASSERT_TRUE(CompareModule(basic_module.get(), fast_module.get())); return true; } diff --git a/src/processor/module_serializer.cc b/src/processor/module_serializer.cc index 5f83842f..5c5ff77c 100644 --- a/src/processor/module_serializer.cc +++ b/src/processor/module_serializer.cc @@ -100,7 +100,7 @@ char* ModuleSerializer::Serialize( unsigned int size_to_alloc = SizeOf(module); // Allocate memory for serialized data. - char *serialized_data = reinterpret_cast(operator new(size_to_alloc)); + char *serialized_data = new char[size_to_alloc]; if (!serialized_data) { BPLOG(ERROR) << "ModuleSerializer: memory allocation failed, " << "size to alloc: " << size_to_alloc; @@ -134,7 +134,7 @@ bool ModuleSerializer::SerializeModuleAndLoadIntoFastResolver( dynamic_cast(iter->second); unsigned int size = 0; - scoped_ptr symbol_data(Serialize(*basic_module, &size)); + scoped_array symbol_data(Serialize(*basic_module, &size)); if (!symbol_data.get()) { BPLOG(ERROR) << "Serialization failed for module: " << basic_module->name_; return false; @@ -193,6 +193,7 @@ char* ModuleSerializer::SerializeSymbolFileData( if (!module->LoadMapFromMemory(buffer.get())) { return NULL; } + buffer.reset(NULL); return Serialize(*(module.get()), size); } diff --git a/src/processor/module_serializer.h b/src/processor/module_serializer.h index 7a05827d..3b440a60 100644 --- a/src/processor/module_serializer.h +++ b/src/processor/module_serializer.h @@ -71,13 +71,13 @@ class ModuleSerializer { // the address of memory chunk. If size != NULL, *size is set to the memory // size allocated for the serialized data. // Caller takes the ownership of the memory chunk (allocated on heap), and - // should call delete instead of delete [] to free it. + // owner should call delete [] to free the memory after use. char* Serialize(const BasicSourceLineResolver::Module &module, unsigned int *size = NULL); // Given the string format symbol_data, produces a chunk of serialized data. - // Caller takes ownership of the serialized data (on heap), and should call - // delete instead of delete [] to free it. + // Caller takes ownership of the serialized data (on heap), and owner should + // call delete [] to free the memory after use. char* SerializeSymbolFileData(const string &symbol_data, unsigned int *size = NULL);