From 373c49b4167c8a41bfb3adfa613866f0ca516e1c Mon Sep 17 00:00:00 2001 From: mmentovai Date: Mon, 27 Nov 2006 21:42:07 +0000 Subject: Eliminate usage of vector<>[0] for 0-sized vectors in processor library (#84). r=bryner http://groups.google.com/group/airbag-dev/browse_thread/thread/8eb9277ac06425e3 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@72 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/processor/minidump.cc | 238 ++++++++++++++++++++++++---------------------- 1 file changed, 126 insertions(+), 112 deletions(-) diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc index d557d0bc..ed2e2df5 100644 --- a/src/processor/minidump.cc +++ b/src/processor/minidump.cc @@ -643,6 +643,9 @@ const u_int8_t* MinidumpMemoryRegion::GetMemory() { return NULL; if (!memory_) { + if (descriptor_->memory.data_size == 0) + return NULL; + if (!minidump_->SeekSet(descriptor_->memory.rva)) return NULL; @@ -920,31 +923,34 @@ bool MinidumpThreadList::Read(u_int32_t expected_size) { return false; } - // TODO(mmentovai): verify rational size! - scoped_ptr threads( - new MinidumpThreads(thread_count, MinidumpThread(minidump_))); + if (thread_count) { + // TODO(mmentovai): verify rational size! + scoped_ptr threads( + new MinidumpThreads(thread_count, MinidumpThread(minidump_))); - for (unsigned int thread_index = 0; - thread_index < thread_count; - ++thread_index) { - MinidumpThread* thread = &(*threads)[thread_index]; + for (unsigned int thread_index = 0; + thread_index < thread_count; + ++thread_index) { + MinidumpThread* thread = &(*threads)[thread_index]; - // Assume that the file offset is correct after the last read. - if (!thread->Read()) - return false; + // Assume that the file offset is correct after the last read. + if (!thread->Read()) + return false; - u_int32_t thread_id; - if (!thread->GetThreadID(&thread_id)) - return false; + u_int32_t thread_id; + if (!thread->GetThreadID(&thread_id)) + return false; - if (GetThreadByID(thread_id)) { - // Another thread with this ID is already in the list. Data error. - return false; + if (GetThreadByID(thread_id)) { + // Another thread with this ID is already in the list. Data error. + return false; + } + id_to_thread_map_[thread_id] = thread; } - id_to_thread_map_[thread_id] = thread; + + threads_ = threads.release(); } - threads_ = threads.release(); thread_count_ = thread_count; valid_ = true; @@ -1275,7 +1281,8 @@ const string* MinidumpModule::GetDebugFilename() { // UTF16ToUTF8 expects a vector, so create a temporary one and // copy the UTF-16 data into it. vector string_utf16(utf16_words); - memcpy(&string_utf16[0], &misc_record->data, bytes); + if (utf16_words) + memcpy(&string_utf16[0], &misc_record->data, bytes); // GetMiscRecord already byte-swapped the data[] field if it contains // UTF-16, so pass false as the swap argument. @@ -1451,29 +1458,32 @@ bool MinidumpModuleList::Read(u_int32_t expected_size) { return false; } - // TODO(mmentovai): verify rational size! - scoped_ptr modules( - new MinidumpModules(module_count, MinidumpModule(minidump_))); + if (module_count) { + // TODO(mmentovai): verify rational size! + scoped_ptr modules( + new MinidumpModules(module_count, MinidumpModule(minidump_))); - for (unsigned int module_index = 0; - module_index < module_count; - ++module_index) { - MinidumpModule* module = &(*modules)[module_index]; + for (unsigned int module_index = 0; + module_index < module_count; + ++module_index) { + MinidumpModule* module = &(*modules)[module_index]; - // Assume that the file offset is correct after the last read. - if (!module->Read()) - return false; + // Assume that the file offset is correct after the last read. + if (!module->Read()) + return false; - u_int64_t base_address = module->base_address(); - u_int64_t module_size = module->size(); - if (base_address == (u_int64_t)-1) - return false; + u_int64_t base_address = module->base_address(); + u_int64_t module_size = module->size(); + if (base_address == static_cast(-1)) + return false; - if (!range_map_->StoreRange(base_address, module_size, module_index)) - return false; + if (!range_map_->StoreRange(base_address, module_size, module_index)) + return false; + } + + modules_ = modules.release(); } - modules_ = modules.release(); module_count_ = module_count; valid_ = true; @@ -1566,46 +1576,49 @@ bool MinidumpMemoryList::Read(u_int32_t expected_size) { return false; } - // TODO(mmentovai): verify rational size! - scoped_ptr descriptors( - new MemoryDescriptors(region_count)); + if (region_count) { + // TODO(mmentovai): verify rational size! + scoped_ptr descriptors( + new MemoryDescriptors(region_count)); - // Read the entire array in one fell swoop, instead of reading one entry - // at a time in the loop. - if (!minidump_->ReadBytes(&(*descriptors)[0], - sizeof(MDMemoryDescriptor) * region_count)) { - return false; - } + // Read the entire array in one fell swoop, instead of reading one entry + // at a time in the loop. + if (!minidump_->ReadBytes(&(*descriptors)[0], + sizeof(MDMemoryDescriptor) * region_count)) { + return false; + } - scoped_ptr regions( - new MemoryRegions(region_count, MinidumpMemoryRegion(minidump_))); + scoped_ptr regions( + new MemoryRegions(region_count, MinidumpMemoryRegion(minidump_))); - for (unsigned int region_index = 0; - region_index < region_count; - ++region_index) { - MDMemoryDescriptor* descriptor = &(*descriptors)[region_index]; + for (unsigned int region_index = 0; + region_index < region_count; + ++region_index) { + MDMemoryDescriptor* descriptor = &(*descriptors)[region_index]; - if (minidump_->swap()) - Swap(&*descriptor); + if (minidump_->swap()) + Swap(descriptor); - u_int64_t base_address = descriptor->start_of_memory_range; - u_int32_t region_size = descriptor->memory.data_size; + u_int64_t base_address = descriptor->start_of_memory_range; + u_int32_t region_size = descriptor->memory.data_size; - // Check for base + size overflow or undersize. A separate size==0 - // check is needed in case base == 0. - u_int64_t high_address = base_address + region_size - 1; - if (region_size == 0 || high_address < base_address) - return false; + // Check for base + size overflow or undersize. A separate size==0 + // check is needed in case base == 0. + u_int64_t high_address = base_address + region_size - 1; + if (region_size == 0 || high_address < base_address) + return false; - if (!range_map_->StoreRange(base_address, region_size, region_index)) - return false; + if (!range_map_->StoreRange(base_address, region_size, region_index)) + return false; + + (*regions)[region_index].SetDescriptor(descriptor); + } - (*regions)[region_index].SetDescriptor(descriptor); + descriptors_ = descriptors.release(); + regions_ = regions.release(); } region_count_ = region_count; - descriptors_ = descriptors.release(); - regions_ = regions.release(); valid_ = true; return true; @@ -2110,7 +2123,7 @@ void MinidumpAirbagInfo::Print() { Minidump::Minidump(const string& path) : header_(), directory_(NULL), - stream_map_(NULL), + stream_map_(new MinidumpStreamMap()), path_(path), fd_(-1), swap_(false), @@ -2147,8 +2160,7 @@ bool Minidump::Read() { // Invalidate cached data. delete directory_; directory_ = NULL; - delete stream_map_; - stream_map_ = NULL; + stream_map_->clear(); valid_ = false; @@ -2195,57 +2207,56 @@ bool Minidump::Read() { if (!SeekSet(header_.stream_directory_rva)) return false; - // TODO(mmentovai): verify rational size! - scoped_ptr directory( - new MinidumpDirectoryEntries(header_.stream_count)); - - // Read the entire array in one fell swoop, instead of reading one entry - // at a time in the loop. - if (!ReadBytes(&(*directory)[0], - sizeof(MDRawDirectory) * header_.stream_count)) - return false; + if (header_.stream_count) { + // TODO(mmentovai): verify rational size! + scoped_ptr directory( + new MinidumpDirectoryEntries(header_.stream_count)); - scoped_ptr stream_map(new MinidumpStreamMap()); + // Read the entire array in one fell swoop, instead of reading one entry + // at a time in the loop. + if (!ReadBytes(&(*directory)[0], + sizeof(MDRawDirectory) * header_.stream_count)) + return false; - for (unsigned int stream_index = 0; - stream_index < header_.stream_count; - ++stream_index) { - MDRawDirectory* directory_entry = &(*directory)[stream_index]; + for (unsigned int stream_index = 0; + stream_index < header_.stream_count; + ++stream_index) { + MDRawDirectory* directory_entry = &(*directory)[stream_index]; - if (swap_) { - Swap(&directory_entry->stream_type); - Swap(&directory_entry->location); - } + if (swap_) { + Swap(&directory_entry->stream_type); + Swap(&directory_entry->location); + } - // Initialize the stream_map map, which speeds locating a stream by - // type. - unsigned int stream_type = directory_entry->stream_type; - switch (stream_type) { - case MD_THREAD_LIST_STREAM: - case MD_MODULE_LIST_STREAM: - case MD_MEMORY_LIST_STREAM: - case MD_EXCEPTION_STREAM: - case MD_SYSTEM_INFO_STREAM: - case MD_MISC_INFO_STREAM: - case MD_AIRBAG_INFO_STREAM: { - if (stream_map->find(stream_type) != stream_map->end()) { - // Another stream with this type was already found. A minidump - // file should contain at most one of each of these stream types. - return false; + // Initialize the stream_map_ map, which speeds locating a stream by + // type. + unsigned int stream_type = directory_entry->stream_type; + switch (stream_type) { + case MD_THREAD_LIST_STREAM: + case MD_MODULE_LIST_STREAM: + case MD_MEMORY_LIST_STREAM: + case MD_EXCEPTION_STREAM: + case MD_SYSTEM_INFO_STREAM: + case MD_MISC_INFO_STREAM: + case MD_AIRBAG_INFO_STREAM: { + if (stream_map_->find(stream_type) != stream_map_->end()) { + // Another stream with this type was already found. A minidump + // file should contain at most one of each of these stream types. + return false; + } + // Fall through to default } - // Fall through to default - } - default: { - // Overwrites for stream types other than those above, but it's - // expected to be the user's burden in that case. - (*stream_map)[stream_type].stream_index = stream_index; + default: { + // Overwrites for stream types other than those above, but it's + // expected to be the user's burden in that case. + (*stream_map_)[stream_type].stream_index = stream_index; + } } } - } - directory_ = directory.release(); - stream_map_ = stream_map.release(); + directory_ = directory.release(); + } valid_ = true; return true; @@ -2391,8 +2402,11 @@ string* Minidump::ReadString(off_t offset) { // TODO(mmentovai): verify rational size! vector string_utf16(utf16_words); - if (!ReadBytes(&string_utf16[0], bytes)) - return NULL; + if (utf16_words) { + if (!ReadBytes(&string_utf16[0], bytes)) { + return NULL; + } + } return UTF16ToUTF8(string_utf16, swap_); } -- cgit v1.2.1