> 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 <ztur...@google.com> wrote: > 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 <amcca...@google.com> > wrote: > >> amccarth added inline comments. >> >> ================ >> Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21 >> @@ +20,3 @@ >> +llvm::StringRef >> +lldb_private::minidump::consumeString(llvm::ArrayRef<uint8_t> &Buffer) { >> + return llvm::StringRef(reinterpret_cast<const char *>(Buffer.data()), >> ---------------- >> zturner wrote: >> > labath wrote: >> > > This is not consistent with the consumeObject function, which also >> drops the consumed bytes from the buffer. >> > 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? >> 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. >> >> Oh, but this isn't used for minidump strings. It looks like it's for >> these Linux proc status fields. >> >> >> https://reviews.llvm.org/D24385 >> >> >> >>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits