This revision was automatically updated to reflect the committed changes.
Closed by commit rL281348: MinidumpParsing: pid, modules, exceptions, strings
(authored by dvlahovski).
Changed prior to commit:
https://reviews.llvm.org/D24385?vs=71154&id=71182#toc
Repository:
rL LLVM
https://review
Looks good here too
On Tue, Sep 13, 2016 at 6:21 AM Pavel Labath wrote:
> labath added a comment.
>
> Looks good as far as I am concerned.
>
>
> https://reviews.llvm.org/D24385
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lis
labath added a comment.
Looks good as far as I am concerned.
https://reviews.llvm.org/D24385
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
dvlahovski updated this revision to Diff 71154.
dvlahovski added a comment.
Making the LinuxProcStatus a smarter class
by not parsing the pid each time when GetPid is called
https://reviews.llvm.org/D24385
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/min
dvlahovski updated this revision to Diff 71151.
dvlahovski marked 3 inline comments as done.
dvlahovski added a comment.
Removed consumeString; Fixed comparisons in unittests; out-of-bounds check in
MinidumpString parsing
https://reviews.llvm.org/D24385
Files:
source/Plugins/Process/minidump
labath added a comment.
Just a couple of tiny comments from me.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:86
@@ +85,3 @@
+llvm::Optional MinidumpParser::GetMinidumpString(uint32_t rva) {
+ auto arr_ref = m_data_sp->GetData().drop_front(rva);
+ return parse
zturner added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:222
@@ -154,1 +221,2 @@
+ return MinidumpExceptionStream::Parse(data);
}
dvlahovski wrote:
> Now that I return an ArrayRef, which is basically a reinterpret
> cast of
dvlahovski added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:222
@@ -154,1 +221,2 @@
+ return MinidumpExceptionStream::Parse(data);
}
Now that I return an ArrayRef, which is basically a reinterpret
cast of a piece of memory t
dvlahovski updated this revision to Diff 71026.
dvlahovski marked 17 inline comments as done.
dvlahovski added a comment.
Herald added subscribers: mgorny, srhines, danalbert, tberghammer.
Changes regarding all of the comments.
Removed llvm::Optionals where it was unneeded.
Parsing also the OS fr
> Even if it's length prefixed, the logic here basically just consumes the
entire buffer, which doesn't seem right
Yes, agreed.
On Fri, Sep 9, 2016 at 5:45 PM, Zachary Turner wrote:
> Even if it's length prefixed, the logic here basically just consumes the
> entire buffer, which doesn't seem ri
Even if it's length prefixed, the logic here basically just consumes the
entire buffer, which doesn't seem right
On Fri, Sep 9, 2016 at 5:43 PM Adrian McCarthy wrote:
> amccarth added inline comments.
>
>
> Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
> @@ +20
amccarth added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef &Buffer) {
+ return llvm::StringRef(reinterpret_cast(Buffer.data()),
zturner wrote:
zturner added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:88-89
@@ +87,4 @@
+MinidumpParser::GetMinidumpString(uint32_t rva) {
+ llvm::ArrayRef arr_ref(m_data_sp->GetBytes() + rva,
+ m_data_sp->GetByteSize() - r
labath added a comment.
A collection of small remarks, mostly to do with style. Mainly, I'd like to
make the tests more strict about what they accept as reasonable values.
A higher level question: Given that we are already in the `minidump` namespace,
do we want all (most) of these symbols to b
dvlahovski added a comment.
Also added parsing code for Minidump strings - the string in the file are
UTF-16 encoded. I used the code from the WinMiniDump plugin and it can extract
a UTF-16 string and convert it to a UTF-8 one.
https://reviews.llvm.org/D24385
___
dvlahovski updated this revision to Diff 70816.
dvlahovski added a comment.
Forgot to run clang-format
https://reviews.llvm.org/D24385
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.c
dvlahovski created this revision.
dvlahovski added reviewers: labath, zturner.
dvlahovski added a subscriber: lldb-commits.
Herald added a subscriber: beanz.
Added parsing of the MiscInfo data stream.
The main member of it that we care about is the process_id
On Linux generated Minidump (from brea
17 matches
Mail list logo