clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed.
Looking great, just a few more fixes. Thanks for making the changes. ================ Comment at: lldb/include/lldb/API/SBProcess.h:348 + /// \param[in] core_style - Specify the style of a core file to save. + /// "minidump" plug-in supports only "eSaveCoreStackOnly". + lldb::SBError SaveCore(const char *file_name, const char *flavor, ---------------- Might not need to mention this in the header file as long as the SBError mentions that it isn't supported in the error message. ================ Comment at: lldb/source/API/SBProcess.cpp:1147 + SaveCoreStyle core_style) { + LLDB_INSTRUMENT_VA(this, file_name, flavor); ---------------- missing core_style in the LLDB_INSTRUMENT_VA macro ================ Comment at: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:47 + # validate savinig via SBProcess + lldb.SBProcess().SaveCore(core_sb, "minidump", "stack") self.assertTrue(os.path.isfile(core)) ---------------- The 3rd argument is being passed by string value which isn't correct. We need to use the enumeration. ================ Comment at: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:47 + # validate savinig via SBProcess + lldb.SBProcess().SaveCore(core_sb, "minidump", "stack") self.assertTrue(os.path.isfile(core)) ---------------- clayborg wrote: > The 3rd argument is being passed by string value which isn't correct. We need > to use the enumeration. We might think about adding a test for "minidump" to save it with a style of "lldb.eSaveCoreFull" and "lldb.eSaveCoreDirtyOnly" and verify we get an error back. If this ever changes, then we can modify the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125325/new/ https://reviews.llvm.org/D125325 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits