JDevlieghere added inline comments.
================ Comment at: lldb/examples/python/crashlog.py:1022 + # 1. Try to use the user-provided target + if options.target_path is not None and options.target_path != "": + target = debugger.CreateTarget(options.target_path) ---------------- ================ Comment at: lldb/examples/python/crashlog.py:1024 + target = debugger.CreateTarget(options.target_path) + # 2. If that didn't work, try to create a target using the symbolicator + if target is None or not target.IsValid(): ---------------- I think if the user provided a target path and that didn't work, we should report and error and give up. ================ Comment at: lldb/examples/python/crashlog.py:1025 + # 2. If that didn't work, try to create a target using the symbolicator + if target is None or not target.IsValid(): target = crashlog.create_target() ---------------- ================ Comment at: lldb/examples/python/crashlog.py:1197 + dest='target_path', + help='the target binary path that should be used for interactive crashlog') return option_parser ---------------- Please make it clear that this is optional and describe the fallback behavior. ================ Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:46 - if not self.target or not self.target.IsValid(): + if not self.target.GetExecutable() or not self.target.IsValid(): + # Return error ---------------- Maybe leave a comment why the executable matters here rather than just the target. ================ Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:47-48 + if not self.target.GetExecutable() or not self.target.IsValid(): + # Return error return ---------------- The comment says return error but we're not returning anything. Should this raise an exception? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129611/new/ https://reviews.llvm.org/D129611 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits