Closed
Bug 768317
Opened 12 years ago
Closed 12 years ago
Don't try to save a reference to the internal buffer of NS_ConvertUTF16toUTF8 in DirPrefObserver
Categories
(MailNews Core :: Address Book, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 16.0
People
(Reporter: rkent, Assigned: rkent)
Details
Attachments
(1 file)
980 bytes,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
My AB unit tests in ExQuilla were acting strangely, and I traced it to this core code, given here with the disassembly: (in nsDirPrefs.cpp): const char *prefname = NS_ConvertUTF16toUTF8(aData).get(); 5B49B8D1 mov eax,dword ptr [aData] 5B49B8D4 push eax 5B49B8D5 lea ecx,[ebp-6Ch] 5B49B8D8 call NS_ConvertUTF16toUTF8::NS_ConvertUTF16toUTF8 (597FAF02h) 5B49B8DD mov ecx,eax 5B49B8DF call nsCString::get (59891470h) 5B49B8E4 mov dword ptr [prefname],eax 5B49B8E7 lea ecx,[ebp-6Ch] 5B49B8EA call NS_ConvertUTF16toUTF8::~NS_ConvertUTF16toUTF8 (597C5375h) DIR_PrefId id = DIR_AtomizePrefName(prefname); What I see in the debugger is that aData makes sense, but *prefname is a long string containing only "Z". Stepping through the assembler, the conversion is done correctly but gets overwritten in NS_ConvertUTF16toUTF8::~NS_ConvertUTF16toUTF8 The code is not correct. You are trying to hold onto the reference to the internal buffer of a CString after the destruction of the CString. The destructor is zeroing that string (possibly because I am running in debug mode) so the net result is not correct. My symptoms of this are usually that the unit test simply stops without crashing, though sometimes I seem to get a breaking error in the DIR_AtomizePrefName call. Not sure if this is an issue during normal operation or not.
Assignee | ||
Comment 1•12 years ago
|
||
The fix is pretty simple, and hopefully non-controversial, so I'll ask for it since I don't have a good workaround. The fix itself is obvious, but it may cause a feature that currently may not work to suddenly start working, which could have its own side effects. Not sure what those might be.
Updated•12 years ago
|
Attachment #636735 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Checked in http://hg.mozilla.org/comm-central/rev/7fa3d8900dc2
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
You need to log in
before you can comment on or make changes to this bug.
Description
•