Closed
Bug 1165731
Opened 10 years ago
Closed 10 years ago
Remove unnecessary SetCapacity call in variant_storage_traits::storage_conversion
Categories
(Core :: SQLite and Embedded Database Bindings, defect)
Core
SQLite and Embedded Database Bindings
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: poiru, Assigned: poiru)
References
Details
Attachments
(1 file)
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.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8606758 -
Flags: review?(bent.mozilla)
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...
Attachment #8606758 -
Flags: review?(bent.mozilla) → review?(mak77)
Comment 3•10 years ago
|
||
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.
Attachment #8606758 -
Flags: review?(mak77) → review+
Comment 4•10 years ago
|
||
I filed bug 1166204 for the fallibleTArray, please reply there.
Comment 6•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Updated•2 months ago
|
Product: Toolkit → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•