1165731 - Remove unnecessary SetCapacity call in variant_storage_traits::storage_conversion

Status: RESOLVED FIXED

(Toolkit :: Storage, defect)

Description

[Biru [:poiru]]

AppendElements already ensures that sufficient capacity is available so the explicit SetCapacity call is not needed. Removing it because code like this will cause -Wunused-result warnings after bug 968520 is done.

Comment 1

[Biru [:poiru]]

Attachment: Remove unnecessary SetCapacity call in variant_storage_traits::storage_conversion

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8606758 [details] [diff] [review]
Remove unnecessary SetCapacity call in variant_storage_traits::storage_conversion

I'll redirect to mak since I'm not familiar with this code really. However, IMO it seems really silly to explicitly use a FallibleTArray and the deliberately ignore the return value...

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8606758 [details] [diff] [review]
Remove unnecessary SetCapacity call in variant_storage_traits::storage_conversion

Review of attachment 8606758 [details] [diff] [review]:
-----------------------------------------------------------------

I think the idea behind setCapacity was an optimization for malloc... but i'd expect AppendElements to do that, since this is not a loop of single appends.
Indeed AppendElements uses EnsureCapacity, and the same is done by SetCapacity.

The fallibleTArray has been introduced in bug 610823 that introduces infallible arrays, but it was a fallible nsTArray before as well.
I think it's invoked in the variant constructor, so I'm not sure we can report any useful error from there. We could OOM crash, but for byte arrays that could also be very large, I don't think that'd be nice to the user. We could assert in debug builds, but that wouldn't prevent release errors.
Maybe we could store a status bool in the constructor, and check it in the getters? Thoughts?

Regardless, that's a bug that should be filed apart from this.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I filed bug 1166204 for the fallibleTArray, please reply there.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5795f6ace557

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5795f6ace557