> 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

Reply via email to