13038 - unexpected heap alloc in nsCAutoString::nsCAutoString(const nsStr&)

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Chris Waterson]

This method creates an unnecessary temporary because it must coerce the |const
nsStr&| to a |const nsCString&| in order to call nsCString::Append(const
nsCString&). I'd recommend either adding nsCString::Append(const nsStr&), or
replacing the call to Append with a direct call to nsStr::Assign()...

Index: nsString.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/ds/nsString.cpp,v
retrieving revision 3.73
diff -c -r3.73 nsString.cpp
*** nsString.cpp	1999/09/02 07:04:54	3.73
--- nsString.cpp	1999/09/02 18:13:29
***************
*** 1775,1781 ****
    mAgent=0;
    nsStr::Initialize(*this,mBuffer,sizeof(mBuffer)-1,0,eOneByte,PR_FALSE);
    AddNullTerminator(*this);
!   Append(aString);
  }


--- 1775,1782 ----
    mAgent=0;
    nsStr::Initialize(*this,mBuffer,sizeof(mBuffer)-1,0,eOneByte,PR_FALSE);
    AddNullTerminator(*this);
!   //Append(aString); -- oops, this'll construct an nsCString to coerce!
!   nsStr::Assign(*this,aString,0,aString.mLength,mAgent);
  }

Comment 1

[Chris Waterson]

rickg: any progress on this? this is actually showing up in profiles that i've
been doing. would you like _me_ to check in the patch?

Comment 2

[rickg]

I've added nsCString::Append(nstr), and I'm testing it. Once I'm confident that
all is well I'll check it in. I haven't seen anything odd yet, so if you want to
add it to your build (temporarily) until I land my change, it *should* work fine
for you.

Comment 3

[rickg]

Fixed by latest update to nsCString. Added nsStr constructor. r=kmcclusk.

Comment 4

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old