58284 - wide string literals created by NS_ConvertASCIItoUCS2

Status: RESOLVED DUPLICATE of bug 104159

(Core :: XPCOM, defect, P3)

Description

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

There are lots of wide string literals that should use NS_LITERAL_STRING that
are being created using NS_ConvertASCIItoUCS2 (which is how NS_LITERAL_STRING
works on platforms that don't support 2-byte L"literal").

The following LXR search should show a nearly complete list:
http://lxr.mozilla.org/seamonkey/search?string=NS_ConvertASCIItoUCS2%28%22

It also shows some examples of other bad string usage.

Comment 1

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

Some other bad (IMO) string usage I've seen in that query:

 - Using AppendWithConversion when only Append is needed
 - nsAutoString()  [although this may sometimes be an out param that is ignored]
 - appending a single-character string instead of a PRUnichar
 - "nsString(NS_ConvertASCIItoUCS2("Server: %s - port %d")).GetUnicode()"

Comment 2

[Peter "jag" Annema]

Attachment: [patch] first take... should build, but please test

Comment 3

[Peter "jag" Annema]

Attachment: [patch] Same patch, but with some more cleaning up

Comment 4

[Peter "jag" Annema]

Attachment: [patch] added |get()| to nsString, replaced |GetBuffer()| calls, updated for os2/nsFilePicker.cpp changes

Comment 5

[Peter "jag" Annema]

Attachment: [patch] Update to nsMessenger.cpp (did a thinko there).

Comment 6

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

Blake accidentally checked in the changes to nsNNTPProtocol.cpp, which caused
the following bustage on speedracer:

nsNNTPProtocol.cpp: In method `int nsNNTPProtocol::DisplayNewsRC()':
nsNNTPProtocol.cpp:3679: ambiguous conversion from `nsCString' to `bool'
nsNNTPProtocol.cpp:3679: candidate conversions include `const char *' and `char *'
nsNNTPProtocol.cpp:3679: `class nsCString' used where a `bool' was expected
nsNNTPProtocol.cpp:3679: ambiguous default type conversion for `operator &&'
nsNNTPProtocol.cpp:3730: warning: negative value `-1' passed as argument 2 of
`nsNNTPProtocol::SetProgressBarPercent(unsigned int, unsigned int)'

I'm not sure if that ever returns false without the change...

Comment 7

[Peter "jag" Annema]

Attachment: [patch] Just the |get()| addition to nsString.h

Comment 8

[Peter "jag" Annema]

Okay, I've updated the patch and split it up. I just attached the |get()|
addition to nsString.h and moving |GetBuffer()| from nsString.cpp to nsString.h,
as soon as my build is done compiling (like, tomorrow) I will attach the actual
conversions to NS_LITERAL_STRING themselves.

Comment 9

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

r=dbaron on the most recent patch, assuming scc super-reviews it, since he's
module owner.

Comment 10

[Peter "jag" Annema]

Attachment: [patch] updated patch of conversion to NS_LITERAL_STRING

Comment 11

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

A few thoughts:
 * you're changing code generated by the DOM idlc.  It would be trivial to fix
the DOM idlc, but it looks like the code it calls takes an nsString& and just
converts it to a C string.  So why bother with either NS_LITERAL_STRING or
NS_ConvertASCIItoUCS2?
 * What happens when the NS_ConvertASCIItoUCS2 was used as a parameter to a
function taking an nsString& (or something similar).  How does that compile?  I
thought those constructors were |explicit|.  So what conversion is used?

Comment 12

[Peter "jag" Annema]

It turns out I'm bitten by this compiling on linux with gcc 2.95.2 because the macro for NS_LITERAL_STRING here is just NS_ConvertASCIItoUCS2. On other platforms it's nsLiteralString which is derived from nsAReadableString and isn't interchangable with nsAutoString (which via via inherits from nsAReadableString) for some uses, so half of this wouldn't have compiled on those platforms. I'll go over these changes closely and will attach a new patch sometime.

Comment 13

[(not reading, please use [e-mail] instead)]

I'm gotten a partial patch from jag for nsNNTPProtocol and I'm going to land it
with other fixes.

Comment 14

[Scott Collins]

sr=scc for the <a
href="http://bugzilla.mozilla.org/showattachment.cgi?attach_id=20389">|.get()|
patch</a> (12/08/00 19:59)

Comment 15

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

NS_ConvertToString is just |#define|d to be NS_ConvertASCIItoUCS2, so we should
also deal with all the cases shown in:

http://lxr.mozilla.org/seamonkey/search?string=NS_ConvertToString%28%22

Comment 16

[Peter "jag" Annema]

Mass move to [e-mail]

Comment 17

[Peter Trudelle]

->moz1.0

Comment 18

[jag (Peter Annema)]

*** This bug has been marked as a duplicate of 104159 ***