jingham added a comment.
SBTarget.Attach, and Launch take SBError references, and they are pretty well
tested. So I don't think that's a concern here.
We don't use the text content of error messages programmatically in lldb.
If you wanted to make these errors actionable, the low level
amccarth added a comment.
My motivation was not to check for different kinds of errors but to ensure that
the error code detail doesn't get lost through the SWIG plumbing. SBErrors
seem to be marshaled properly when used as return values, but this is the first
one I've seen returned by
The intention in the scope of this change is just to check that the new
overload is exposed correctly through the Python API.
In general, guaranteeing specific error codes (messages?) is likely
impractical:
1. It's not always easy to do the proper checks (ex. for 'file-not-found'
you'd actually
lemo added a comment.
The intention in the scope of this change is just to check that the new
overload is exposed correctly through the Python API.
In general, guaranteeing specific error codes (messages?) is likely
impractical:
1. It's not always easy to do the proper checks (ex. for
amccarth added inline comments.
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py:78
+self.assertFalse(self.process, PROCESS_IS_VALID)
+self.assertTrue(error.Fail())
+
Is it worth
On Mon, 11 Jun 2018 at 22:00, Leonard Mosescu via lldb-commits
wrote:
>>
>> remove space before (
>
>
> I'd be happy to make the change, but looking at the rest of the file it seems
> that almost everything uses the opposite convention: Foo (...).
>
I guess that's because the .i files escaped
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334439: Add a new SBTarget::LoadCore() overload which
surfaces errors if the load fails (authored by lemo, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
lemo marked 3 inline comments as done.
lemo added a comment.
spaces removed :)
https://reviews.llvm.org/D48049
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
lemo updated this revision to Diff 150844.
https://reviews.llvm.org/D48049
Files:
include/lldb/API/SBTarget.h
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
scripts/interface/SBTarget.i
source/API/SBTarget.cpp
Index: source/API/SBTarget.cpp
lemo updated this revision to Diff 150843.
https://reviews.llvm.org/D48049
Files:
include/lldb/API/SBTarget.h
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
scripts/interface/SBTarget.i
source/API/SBTarget.cpp
Index: source/API/SBTarget.cpp
Should not have spaces before any ( characters. What you typed looks good.
> On Jun 11, 2018, at 2:00 PM, Leonard Mosescu wrote:
>
> remove space before (
>
> I'd be happy to make the change, but looking at the rest of the file it seems
> that almost everything uses the opposite convention:
lemo added subscribers: amccarth, clayborg, zturner.
lemo added a comment.
> remove space before (
I'd be happy to make the change, but looking at the rest of the file it
seems that almost everything uses the opposite convention: Foo (...).
So, to make sure I'm making the right change, is this
>
> remove space before (
>
I'd be happy to make the change, but looking at the rest of the file it
seems that almost everything uses the opposite convention: Foo (...).
So, to make sure I'm making the right change, is this how it should look?
lldb::SBProcess
LoadCore(const char
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Remove extra spaces before ( and good to go.
Comment at: scripts/interface/SBTarget.i:282
lldb::SBProcess
-LoadCore(const char *core_file);
-
+LoadCore
lemo updated this revision to Diff 150835.
lemo added a comment.
Adding test cases for the new overload.
https://reviews.llvm.org/D48049
Files:
include/lldb/API/SBTarget.h
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lemo created this revision.
lemo added reviewers: zturner, clayborg, amccarth.
There was no way to find out what's wrong if SBProcess SBTarget::LoadCore(const
char *core_file) failed.
Additionally, the implementation was unconditionally setting sb_process, so it
wasn't even possible to check
16 matches
Mail list logo
