clayborg added a comment. LGTM. A few inline nits, but nothing major. I only question if "RecordedProcess" is the right name for the base class for a core file/trace file, but don't have much of a better name. Maybe SerializedProcess? SavedProcess? Don't we usually have Pavel in on these reviews?
================ Comment at: lldb/include/lldb/Target/RecordedProcess.h:18 +/// Base class for all processes that don't represent a live process, such as +/// coredumps or processes traced in the past. +class RecordedProcess : public Process { ---------------- Might want to comment something like "common lldb_private::Process virtual functions overrides that are common between these kinds of processes can have default implementations in this class". ================ Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:209 GetTarget().SetArchitecture(target_arch); - + SetUnixSignals(UnixSignals::Create(GetArchitecture())); ---------------- undo space only change ================ Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:246-247 if (exe_module_spec.GetFileSpec()) { - exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, - true /* notify */); + exe_module_sp = + GetTarget().GetOrCreateModule(exe_module_spec, true /* notify */); if (exe_module_sp) ---------------- undo space only change ================ Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:766 /// - NT_FILE - Files mapped into memory -/// +/// /// Additionally, for each thread in the process the core file will contain at ---------------- undo space only change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90729/new/ https://reviews.llvm.org/D90729 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits