From feea0f7672e5e7ac33ebbf6acb13e580f8b6b136 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 14 Jan 2025 23:39:39 -0500 Subject: [PATCH] use const string& in more places to avoid unnecessary copy constructors Coverity has started checking for wasteful copies where move's would work which has pointed out a bunch of places where we use "string" in a function prototype even though it's always read-only. Convert all those over to "const string&" for simple performance improvements. Change-Id: I264b19c1de8d2d0aa69032bff44cea95cc057a70 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/6173849 Reviewed-by: Nelson Billing --- .../handler/exception_handler_unittest.cc | 2 +- src/client/mac/handler/dynamic_images.h | 2 +- src/common/linux/dump_symbols.cc | 2 +- .../basic_source_line_resolver_unittest.cc | 2 +- src/processor/disassembler_objdump.cc | 20 ++++++++++++------- .../fast_source_line_resolver_unittest.cc | 2 +- src/processor/windows_frame_info.h | 4 ++-- src/tools/windows/converter_exe/converter.cc | 2 +- 8 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index b2d8d4681..d512bd8d7 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -378,7 +378,7 @@ static bool InstallRaiseSIGKILL() { static void CrashWithCallbacks(ExceptionHandler::FilterCallback filter, ExceptionHandler::MinidumpCallback done, - string path) { + const string& path) { ExceptionHandler handler( MinidumpDescriptor(path), filter, done, NULL, true, -1); // Crash with the exception handler in scope. diff --git a/src/client/mac/handler/dynamic_images.h b/src/client/mac/handler/dynamic_images.h index b5af75848..4799fedfc 100644 --- a/src/client/mac/handler/dynamic_images.h +++ b/src/client/mac/handler/dynamic_images.h @@ -110,7 +110,7 @@ class DynamicImage { DynamicImage(uint8_t* header, // data is copied size_t header_size, // includes load commands uint64_t load_address, - string file_path, + const string& file_path, uintptr_t image_mod_date, mach_port_t task, cpu_type_t cpu_type) diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index 55fa98469..316d58a30 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -826,7 +826,7 @@ class LoadSymbolsInfo { string debuglink_file() const { return debuglink_file_; } - void set_debuglink_file(string file) { + void set_debuglink_file(const string& file) { debuglink_file_ = file; } diff --git a/src/processor/basic_source_line_resolver_unittest.cc b/src/processor/basic_source_line_resolver_unittest.cc index a73ded8b7..c496cbb97 100644 --- a/src/processor/basic_source_line_resolver_unittest.cc +++ b/src/processor/basic_source_line_resolver_unittest.cc @@ -60,7 +60,7 @@ using google_breakpad::SymbolParseHelper; class TestCodeModule : public CodeModule { public: - TestCodeModule(string code_file) : code_file_(code_file) {} + TestCodeModule(const string& code_file) : code_file_(code_file) {} virtual ~TestCodeModule() {} virtual uint64_t base_address() const { return 0; } diff --git a/src/processor/disassembler_objdump.cc b/src/processor/disassembler_objdump.cc index 9f9569a5e..3e0b6d196 100644 --- a/src/processor/disassembler_objdump.cc +++ b/src/processor/disassembler_objdump.cc @@ -73,7 +73,8 @@ bool IsOperandSize(const string& token) { return false; } -bool GetSegmentAddressX86(const DumpContext& context, string segment_name, +bool GetSegmentAddressX86(const DumpContext& context, + const string& segment_name, uint64_t& address) { if (segment_name == "ds") { address = context.GetContextX86()->ds; @@ -91,7 +92,8 @@ bool GetSegmentAddressX86(const DumpContext& context, string segment_name, return true; } -bool GetSegmentAddressAMD64(const DumpContext& context, string segment_name, +bool GetSegmentAddressAMD64(const DumpContext& context, + const string& segment_name, uint64_t& address) { if (segment_name == "ds") { address = 0; @@ -105,7 +107,8 @@ bool GetSegmentAddressAMD64(const DumpContext& context, string segment_name, return true; } -bool GetSegmentAddress(const DumpContext& context, string segment_name, +bool GetSegmentAddress(const DumpContext& context, + const string& segment_name, uint64_t& address) { if (context.GetContextCPU() == MD_CONTEXT_X86) { return GetSegmentAddressX86(context, segment_name, address); @@ -117,7 +120,8 @@ bool GetSegmentAddress(const DumpContext& context, string segment_name, } } -bool GetRegisterValueX86(const DumpContext& context, string register_name, +bool GetRegisterValueX86(const DumpContext& context, + const string& register_name, uint64_t& value) { if (register_name == "eax") { value = context.GetContextX86()->eax; @@ -145,7 +149,8 @@ bool GetRegisterValueX86(const DumpContext& context, string register_name, return true; } -bool GetRegisterValueAMD64(const DumpContext& context, string register_name, +bool GetRegisterValueAMD64(const DumpContext& context, + const string& register_name, uint64_t& value) { if (register_name == "rax") { value = context.GetContextAMD64()->rax; @@ -193,7 +198,8 @@ bool GetRegisterValueAMD64(const DumpContext& context, string register_name, // success. // Support for non-full-size registers not implemented, since we're only using // this to evaluate address expressions. -bool GetRegisterValue(const DumpContext& context, string register_name, +bool GetRegisterValue(const DumpContext& context, + const string& register_name, uint64_t& value) { if (context.GetContextCPU() == MD_CONTEXT_X86) { return GetRegisterValueX86(context, register_name, value); @@ -484,4 +490,4 @@ bool DisassemblerObjdump::CalculateDestAddress(const DumpContext& context, return CalculateAddress(context, dest_, address); } -} // namespace google_breakpad \ No newline at end of file +} // namespace google_breakpad diff --git a/src/processor/fast_source_line_resolver_unittest.cc b/src/processor/fast_source_line_resolver_unittest.cc index 08340c15a..5a24d2870 100644 --- a/src/processor/fast_source_line_resolver_unittest.cc +++ b/src/processor/fast_source_line_resolver_unittest.cc @@ -71,7 +71,7 @@ using google_breakpad::scoped_ptr; class TestCodeModule : public CodeModule { public: - explicit TestCodeModule(string code_file) : code_file_(code_file) {} + explicit TestCodeModule(const string& code_file) : code_file_(code_file) {} virtual ~TestCodeModule() {} virtual uint64_t base_address() const { return 0; } diff --git a/src/processor/windows_frame_info.h b/src/processor/windows_frame_info.h index 4014a1a99..346bc4d4b 100644 --- a/src/processor/windows_frame_info.h +++ b/src/processor/windows_frame_info.h @@ -95,7 +95,7 @@ struct WindowsFrameInfo { uint32_t set_local_size, uint32_t set_max_stack_size, int set_allocates_base_pointer, - const string set_program_string) + const string& set_program_string) : type_(type), valid(VALID_ALL), prolog_size(set_prolog_size), @@ -111,7 +111,7 @@ struct WindowsFrameInfo { // a string. Returns NULL if parsing fails, or a new object // otherwise. type, rva and code_size are present in the STACK line, // but not the StackFrameInfo structure, so return them as outparams. - static WindowsFrameInfo *ParseFromString(const string string, + static WindowsFrameInfo *ParseFromString(const string& string, int& type, uint64_t& rva, uint64_t& code_size) { diff --git a/src/tools/windows/converter_exe/converter.cc b/src/tools/windows/converter_exe/converter.cc index 6d7664774..e7b21862e 100644 --- a/src/tools/windows/converter_exe/converter.cc +++ b/src/tools/windows/converter_exe/converter.cc @@ -643,7 +643,7 @@ static void ConvertMissingSymbolFile(const MissingSymbolInfo& missing_info, // Reads the contents of file |file_name| and populates |contents|. // Returns true on success. -static bool ReadFile(string file_name, string* contents) { +static bool ReadFile(const string& file_name, string* contents) { char buffer[1024 * 8]; FILE* fp = fopen(file_name.c_str(), "rt"); if (!fp) {