1381725 - Consider removing NS_xstr*dup functions

Status: NEW

(Core :: XPCOM, enhancement, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Now that we don't have multiple allocators floating around, we should be able to replace these with other things.  Perhaps even just use of our string classes.

Comment 1

[Nathan Froyd [:froydnj]]

These appear--at least for NS_strdup--to be largely associated with IDL methods that return `string` as opposed to `ACString` or similar.  njn was interested in cleaning up IDL files like this, perhaps he'd be interested in tackling this?

Comment 2

[Nicholas Nethercote [inactive]]

The 8-bit NS_str[n]dup can be trivially replaced with moz_xstr[n]dup, or (harder) our string classes.

The 16-bit ones don't have an immediate replacement. http://searchfox.org/mozilla-central/search?q=symbol:_Z9NS_strdupPKDs&redirect=false suggests that the 16-bit NS_strdup's uses mostly aren't related to wstring in IDL files.

Comment 3

[Nicholas Nethercote [inactive]]

I worked out which ones relate to IDL methods/attributes and can be changed easily.

string (8-bit):

- dom/interfaces/base/nsIDOMWindowUtils.idl: nsIDOMWindowUtils::getClassName

- image/imgIRequest.idl: imgIRequest::mimeType

- js/xpconnect/idl/xpccomponents.idl: nsIXPCComponents_Utils::getClassName

- js/xpconnect/idl/xpcjsid.idl: nsIJSID::{name,number}

- netwerk/cache/nsICacheVisitor.idl: nsICacheEntryInfo::{clientID,deviceID}
  nsICacheDeviceInfo::description
  (Tom is already dealing with these in bug 1410620.)

- netwerk/cache/nsICacheEntryDescriptor.idl: 
  nsICacheEntryDescriptor::getMetaDataElement,

- rdf/base/nsIRDFDataSource.idl: nsIRDFSource::URI

- xpcom/components/nsCategoryManager.cpp: nsICategoryManager::getCategoryEntry

wstring:

- netwerk/streamconv/mozITXTToHTMLConv.idl: mozITXTToHTMLConv::scanTXT

- rdf/base/nsIRDFLiteral.idl: nsIRDFLiteral::Value

That leaves a few more. A few can be removed by switching C strings to Gecko strings. A few more are harder, because they involve strings within unions, or outparam arrays in IDL methods, or hash table keys. Among those, the 8-bit ones can just be converted to moz_xstrdup, but the 16-bit ones don't have an alternative.

As for NS_strndup(), it has a single use in GetOrInternStringMatcher::match(). Unfortunately, that function is templated to work with both char and char16_t, so it needs both the 8-bit and 16-bit versions of NS_strndup() to work. It could probably be changed to use nsMemory::Clone() instead, but it's not clear that's an improvement.

In summary, getting rid of most of the uses is easy. Getting rid of the last few is harder.

Comment 4

[Nicholas Nethercote [inactive]]

Some of the .idl files mentioned have other string/wstring uses. If we are fixing the listed ones, we might as will fix the others too. (Except for the array outparams, for which we can't currently use AUTF8String/AString.)

Comment 5

[Nicholas Nethercote [inactive]]

Note that these functions were renamed as `NS_xstr*dup`, to reflect their infallibility.