Closed Bug 504660 Opened 11 years ago Closed 11 years ago

EnsureMutable(0) doesn't?

Categories

(Core :: String, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

Details

Attachments

(1 file)

EnsureMutable(0) calls SetLength(0) which calls SetCapacity(0) which sets mData to sEmptyBuffer which isn't mutable.

Would it possibly be more realistic to use ReplacePrep?

if (newLen == size_type(-1) || newLen == mLength)
  {
    if (mFlags & (F_FIXED | F_OWNED))
      return PR_TRUE;
    if ((mFlags & F_SHARED) && !nsStringBuffer::FromData(mData)->IsReadonly())
      return PR_TRUE;
  }
if (newLen < mLength)
  return ReplacePrep(0, newLen, mLength - newLen, 0);
return ReplacePrep(0, mLength, 0, newLen - mLength);
EnsureMutable(0) doesn't actually need to do anything: you can't actually mutate a 0-length string... seems to me it works as intended.
Some functions, such as nsUnescapeCount, overwrite a zero-length string...
How does that work? Isn't that a buffer overflow?
No, because they only want to overwrite it with another zero-length string.
So specifically they write a null terminator? I rather dislike that, but if that's the case maybe we can just make EmptyString use a non-const buffer?
Attached patch Like this?Splinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #389329 - Flags: review?(benjamin)
Attachment #389329 - Flags: review?(benjamin) → review+
Pushed changeset d347a454b338 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Seems like it's worth adding a test for this to TestStrings, since it's an odd use case.
You need to log in before you can comment on or make changes to this bug.