<div dir="ltr"><div>On Fri, Oct 17, 2014 at 3:59 PM, Tim Northover <span dir="ltr"><<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Rui,<br>
<br>
<sniiiiiiiiiiiiiiiip><br>
<span class="">>> +  void addCommonEncodings(std::vector<uint32_t> &commonEncodings) {<br>
>> +    using normalized::write32;<br>
>> +<br>
>> +    _contents.resize(_commonEncodingsOffset +<br>
>> +                     commonEncodings.size() * sizeof(uint32_t));<br>
>> +    int32_t *commonEncodingsArea =<br>
>> +        (int32_t *)&_contents[_commonEncodingsOffset];<br>
><br>
><br>
> It doesn't seem to be guaranteed that _commonEncodingsOffset is a valid<br>
> index. If I add<br>
><br>
>   assert(_commonEncodingsOffset < _contents.size());<br>
><br>
> many tests start failing.<br>
<br>
</span>I'd actually expect _commonEncodingsOffset == _contents.size() in all<br>
cases (and an assert seems to confirm this). We've added the top-level<br>
header, the next bytes (which we' rather like to be contiguous for<br>
efficiency reasons) are the common encodings, so we resize the buffer<br>
to give them room.</blockquote><div><br></div><div>commonEncodings.size() can be zero it seems. In that case _contents become the same size as _commonEncodingsOffset.</div><div><br></div><div>I don't think taking an address of v[n] where n is an integer and v is a std::vector of size n is valid. If you run this test on Windows with debug build, it will actually fail because of an assertion in the standard library.</div></div></div></div>