wide string literals created by NS_ConvertASCIItoUCS2

RESOLVED DUPLICATE of bug 104159

Status

()

P3
normal
RESOLVED DUPLICATE of bug 104159
18 years ago
14 years ago

People

(Reporter: dbaron, Assigned: jag-mozilla)

Tracking

({meta})

Trunk
mozilla1.0
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(6 attachments)

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.
Keywords: meta
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

18 years ago
Created attachment 19116 [details] [diff] [review]
[patch] first take... should build, but please test

Comment 3

18 years ago
Created attachment 19141 [details] [diff] [review]
[patch] Same patch, but with some more cleaning up

Comment 4

18 years ago
Created attachment 19207 [details] [diff] [review]
[patch] added |get()| to nsString, replaced |GetBuffer()| calls, updated for os2/nsFilePicker.cpp changes

Comment 5

18 years ago
Created attachment 19224 [details] [diff] [review]
[patch] Update to nsMessenger.cpp (did a thinko there).
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

18 years ago
Created attachment 20389 [details] [diff] [review]
[patch] Just the |get()| addition to nsString.h

Comment 8

18 years ago
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.
r=dbaron on the most recent patch, assuming scc super-reviews it, since he's
module owner.

Comment 10

18 years ago
Created attachment 20425 [details] [diff] [review]
[patch] updated patch of conversion to NS_LITERAL_STRING
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

18 years ago
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.
I'm gotten a partial patch from jag for nsNNTPProtocol and I'm going to land it
with other fixes.

Comment 14

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

18 years ago
Mass move to jaggernaut@netscape.com
Assignee: jag → jaggernaut

Comment 17

18 years ago
->moz1.0
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 18

17 years ago

*** This bug has been marked as a duplicate of 104159 ***
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → DUPLICATE

Updated

14 years ago
Component: XP Miscellany → String
You need to log in before you can comment on or make changes to this bug.