Open Bug 1381725 Opened 7 years ago Updated 2 years ago

Consider removing NS_xstr*dup functions

Categories

(Core :: XPCOM, enhancement, P3)

53 Branch
enhancement

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug)

Details

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.
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?
Flags: needinfo?(n.nethercote)
Priority: -- → P3
Depends on: 1410518
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.
Flags: needinfo?(n.nethercote)
Depends on: 1410620
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.
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.)
Note that these functions were renamed as `NS_xstr*dup`, to reflect their infallibility.
Summary: Consider removing NS_str*dup functions → Consider removing NS_xstr*dup functions
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.