282814 - nsAutoBuffer::EnsureElemCapacity is broken

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

nsAutoBuffer::EnsureElemCapacity is broken in 2 ways:
 1) it nsMemory::Frees a pointer than has already been nsMemory::Realloced
 2) It does not copy the data when it switches from stack to alloced (which the
comments imply is OK)

Note that fixing (2) costs performance -- an alternative is to document and
enforce that the capacity can only be set once.

Comment 1

[Eli Friedman]

(In reply to comment #0)
>  2) It does not copy the data when it switches from stack to alloced (which the
> comments imply is OK)
> 
> Note that fixing (2) costs performance -- an alternative is to document and
> enforce that the capacity can only be set once.

Another alternative would be to document that data isn't preserved through calls to EnsureElemCapacity; none of the current users depend on the data being preserved, but a few callers depend on being able to change the capacity multiple times.

Comment 2

[Eli Friedman]

Attachment: Patch

I think this approach is correct.

Comment 3

[Eli Friedman]

Attachment: Correct patch

Oops, sorry about that, I accidentally attached the wrong thing.

Comment 4

[Stuart Parmenter]

shouldn't you just use malloc/free/realloc here instead of nsMemory?

Comment 5

[Benjamin Smedberg]

Comment on attachment 278667 [details] [diff] [review]
Correct patch

bonus points for changing nsMemory::Alloc and friends to NS_Alloc and friends while you're there.

Comment 6

[Eli Friedman]

Checked in with change to NS_Alloc and friends.