Last Comment Bug 603243 - remove pitfall nsTString_CharT(char_type c) constructor
: remove pitfall nsTString_CharT(char_type c) constructor
Product: Core
Classification: Components
Component: String (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla11
Assigned To: Atul Aggarwal
: Nathan Froyd [:froydnj]
Depends on:
  Show dependency treegraph
Reported: 2010-10-10 20:08 PDT by Vladimir Vukicevic [:vlad] [:vladv]
Modified: 2011-12-07 12:21 PST (History)
5 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (952 bytes, patch)
2011-11-16 09:10 PST, Atul Aggarwal
dbaron: review+
Details | Diff | Splinter Review

Description User image Vladimir Vukicevic [:vlad] [:vladv] 2010-10-10 20:08:52 PDT
This constructor is a great pitfall, because it's very tempting to write code like this:

  int len = ...;
  nsCAutoString str(len);

  str(len) doesn't even show a warning, which I don't quite understand, since it's a narrower type.  Would be nice to run an analysis to see where the (char) form is used and if we can just get rid of them, then just get rid of this constructor -- or perhaps make it do the more useful thing (capacity len, length 0).
Comment 1 User image Boris Zbarsky [:bz] (still a bit busy) 2010-10-11 07:56:04 PDT
I just tried commenting this ctor out, as well as making it private, and both compile fine on debug Linux.  Want to just do a tryserver run with it private or removed, and if that passes push it?
Comment 2 User image Craig Topper 2010-10-11 20:43:18 PDT
It's used by xpcom/tests/TestTArray.cpp but that appears to only be included in the Makefile in a disable libxul build.
Comment 3 User image Vladimir Vukicevic [:vlad] [:vladv] 2010-10-14 08:05:02 PDT
So if it's not actually used -- what about actually making it do the more useful thing, making it equivalent to str.SetCapacity(len) ?  Though you'd still need to remember to call SetLength after writing to it, so maybe just getting rid of it is best.

I'll make a patch and fix up TestTArray.cpp.
Comment 4 User image Atul Aggarwal 2011-11-16 09:10:05 PST
Created attachment 574912 [details] [diff] [review]
Patch v1

Removing the culprit ctor. I didn't get any failure so I am assuming TestTArray.cpp file has changed.
Comment 5 User image David Baron :dbaron: ⌚️UTC-8 2011-12-05 13:47:09 PST
Comment on attachment 574912 [details] [diff] [review]
Patch v1

Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2011-12-06 21:36:32 PST
Comment 7 User image Matt Brubeck (:mbrubeck) 2011-12-07 12:21:24 PST

Note You need to log in before you can comment on or make changes to this bug.