<div dir="ltr"><span style="font-size:16px">> Even if it's length prefixed, the logic here basically just consumes the entire buffer, which doesn't seem right</span><br><div><span style="font-size:16px"><br></span></div><div><span style="font-size:16px">Yes, agreed.</span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 9, 2016 at 5:45 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Even if it's length prefixed, the logic here basically just consumes the entire buffer, which doesn't seem right<div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 9, 2016 at 5:43 PM Adrian McCarthy <<a href="mailto:amccarth@google.com" target="_blank">amccarth@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">amccarth added inline comments.<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
================<br class="m_8640984112689246742gmail_msg">
Comment at: source/Plugins/Process/<wbr>minidump/MinidumpTypes.cpp:21<br class="m_8640984112689246742gmail_msg">
@@ +20,3 @@<br class="m_8640984112689246742gmail_msg">
+llvm::StringRef<br class="m_8640984112689246742gmail_msg">
+lldb_private::minidump::<wbr>consumeString(llvm::ArrayRef<<wbr>uint8_t> &Buffer) {<br class="m_8640984112689246742gmail_msg">
+  return llvm::StringRef(reinterpret_<wbr>cast<const char *>(Buffer.data()),<br class="m_8640984112689246742gmail_msg">
----------------<br class="m_8640984112689246742gmail_msg">
zturner wrote:<br class="m_8640984112689246742gmail_msg">
> labath wrote:<br class="m_8640984112689246742gmail_msg">
> > This is not consistent with the consumeObject function, which also drops the consumed bytes from the buffer.<br class="m_8640984112689246742gmail_msg">
> Is this logic correct?  A buffer may be arbitrarily large and have more data in it besides the string.  Perhaps you need to search forward for a null terminator, then only return that portion of the string, then drop that many bytes (plus the null terminator) from the input buffer?<br class="m_8640984112689246742gmail_msg">
Minidump strings aren't zero-terminated.  They're counted (in UTF16 code units).  So this would have to read the count and "consume" the appropriate number of bytes.<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
Oh, but this isn't used for minidump strings.  It looks like it's for these Linux proc status fields.<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
<a href="https://reviews.llvm.org/D24385" rel="noreferrer" class="m_8640984112689246742gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D24385</a><br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
<br class="m_8640984112689246742gmail_msg">
</blockquote></div>
</div></div></blockquote></div><br></div>