clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed.
See inlined comments. ================ Comment at: include/lldb/API/SBProcess.h:344-346 @@ -343,1 +343,5 @@ + // Save the state of the process in a core file (or mini dump on Windows). + lldb::SBError + SaveCore(const char *file_name); + ---------------- I don't want any llvm in our public API. Especially llvm::StringRef as it likes to assert when people use it incorrectly (construct one with nullptr and it will assert). So no llvm or clang in our public API. ================ Comment at: source/API/SBProcess.cpp:1433-1437 @@ +1432,7 @@ +{ + ProcessSP process_sp(GetSP()); + lldb::SBFileSpec core_file(file_name); + lldb::SBError error; + error.SetError(PluginManager::SaveCore(process_sp, core_file.get())); + return error; +} ---------------- You need to check your process_sp before using it since it comes from a std::weak_ptr<>. You also need to take the process run lock. You also don't need to create a lldb::SBFileSpec since you actually need a lldb_private::FileSpec. The code should be: ``` lldb::SBError SBProcess::SaveCore(const char *file_name) { lldb::SBError error; if (process_sp) { Mutex::Locker api_locker (process_sp->GetTarget().GetAPIMutex()); FileSpec core_file(file_name, false); error.ref() = PluginManager::SaveCore(process_sp, core_file.get()); } else error.SetErrorString ("SBProcess is invalid"); return error; } ``` ================ Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFMiniDump.cpp:52 @@ +51,3 @@ +#endif + return false; +} ---------------- You should fill in the error and probably use a #else clause: #else error.SetErrorString("windows mini dumps are only supported on native windows machines"); return false; #endif http://reviews.llvm.org/D14793 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits