nsTArray has various Append methods, in this case just using the infallible
AppendElements w/o out a SetCapacity call would do the job. Another option
would be to use SetLength which would default initialize the new elements.
If we're trying to make things fast-but-safeish in this case, the preferred
way is probably:
auto itr = AppendElements(length, fallible);
if (!itr) ...
// no bounds checking
for (...; i++, itr++)
auto& mSocket = *itr;
// bounds checking
for (...)
auto& mSocket = *sockets[i];
In general I agree the pattern of fallibly allocating and then fallibly
appending w/o checking the return value is a bit silly. Perhaps we should
just mark the fallible version MUST_USE? It looks like it's commented out
for some reason (probably a ton of bustage).
-e
On Tue, Aug 1, 2017 at 12:18 PM, Kris Maglione <kmagli...@mozilla.com>
wrote:
> On Tue, Aug 01, 2017 at 12:31:24PM -0400, Alexis Beingessner wrote:
>
>> I was recently searching through our codebase to look at all the ways we
>> use fallible allocations, and was startled when I came across several
>> lines
>> that looked like this:
>>
>> dom::SocketElement &mSocket = *sockets.AppendElement(fallible);
>> ...
>> So in isolation this code is saying "I want to handle allocation failure"
>> and then immediately not doing that and just dereferencing the result.
>> This
>> turns allocation failure into Undefined Behaviour, rather than a process
>> abort.
>>
>> Thankfully, all the cases where I found this were preceded by something
>> like the following:
>>
>> uint32_t length = socketData->mData.Length();if
>> (!sockets.SetCapacity(length, fallible)) { JS_ReportOutOfMemory(cx);
>> return NS_ERROR_OUT_OF_MEMORY;}for (uint32_t i = 0; i <
>> socketData->mData.Length(); i++) { dom::SocketElement &mSocket =
>> *sockets.AppendElement(fallible);
>>
>>
>>
>> So really, the fallible AppendElement *is* infallible, but we're just
>> saying "fallible" anyway. I find this pattern concerning for two reasons:
>>
>
> The MFBT Vector class handles this with a set of infallibleAppend[1]
> methods that assert that space has been reserved for the new elements
> before they're called. If we're using this pattern elsewhere without the
> same safeties, either those places should probably switch to using Vectors,
> or we should probably add similar checked infallible methods to nsTArray.
>
>
> [1]: http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b2092
> 9b56489e2e55438de81fa/mfbt/Vector.h#707-733
>
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
