601515 - Avoid C-casts in updater code

Status: NEW

(Toolkit :: Application Update, defect)

Description

[Justin Dolske [:Dolske]]

Bug 601469 describes a fix to the updater code that was caused by trying to use a pointer to UTF16 data as ASCII.

Normally the compiler could catch this kind of thing, except the C-style casts in this code (to deal with "const" fixup) were suppressing the error. Some C++ const_cast<> magic would be safer.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1 (WIP)

Day late and a dollar short.

WIP, untested outside of compiling on Windows. Results in the following expected error when the bug 601469 patch is removed:

...
nsUpdateDriver.cpp
d:/mozilla-central/obj/toolkit/xre/../../../toolkit/xre/nsUpdateDriver.cpp(454) : d:/mozilla-central/obj/toolkit/xre/../../../toolkit/xre/nsUpdateDriver.cpp(454) : error C2440: '=' : cannot convert from 'const nsAString_internal::char_type *' to 'const char *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
...

Wasn't sure if it was cleaner to make argv const and just have one const_cast<> in the WinLaunchChild() call [really? that's not const?!], or keep argv non-const, and const_cast<> each nsCString.get() when assigned to argv[n]. I did the former.

Should also look through the actual app updater code for any similar issues.

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 480528 [details] [diff] [review]
Patch v.1 (WIP)

Would be nice if the string conversion dance were removed
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsWindowsRestart.cpp#256

but that should be a separate bug

Comment 3

[Justin Dolske [:Dolske]]

(clearing assignment of bugs I'm no long planning to work on)