Closed
Bug 945245
Opened 11 years ago
Closed 11 years ago
Fix misc char16_t/wchar_t mismatches.
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla28
Tracking | Status | |
---|---|---|
firefox28 | --- | fixed |
People
(Reporter: jacek, Assigned: jacek)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file)
28.91 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
This is a followup of bug 928351 containing simple tree-wide changes. Most of such changes have their own bugs, but if a component has just one or two lines of changes, this seems pointless.
Attachment #8341075 -
Flags: review?(ehsan)
Comment 1•11 years ago
|
||
Comment on attachment 8341075 [details] [diff] [review] patch.diff Review of attachment 8341075 [details] [diff] [review]: ----------------------------------------------------------------- ::: embedding/components/printingui/src/win/nsPrintDialogUtil.cpp @@ +687,5 @@ > HGLOBAL hGlobalDevMode = nullptr; > > HANDLE hPrinter = nullptr; > // const cast kludge for silly Win32 api's > + LPWSTR printName = (LPWSTR)aPrintName.get(); Please use reinterpret_cast here. @@ +805,5 @@ > if (printerName.IsEmpty()) { > GetDefaultPrinterNameFromGlobalPrinters(printerName); > } else { > HANDLE hPrinter = nullptr; > + if(!::OpenPrinterW((wchar_t*)printerName.get(), &hPrinter, nullptr)) { Ditto. You may also need a separate const_cast too.
Attachment #8341075 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #1) > Comment on attachment 8341075 [details] [diff] [review] > patch.diff > > Review of attachment 8341075 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: embedding/components/printingui/src/win/nsPrintDialogUtil.cpp > @@ +687,5 @@ > > HGLOBAL hGlobalDevMode = nullptr; > > > > HANDLE hPrinter = nullptr; > > // const cast kludge for silly Win32 api's > > + LPWSTR printName = (LPWSTR)aPrintName.get(); > > Please use reinterpret_cast here. The problem here is that reinterpret_cast requires pointer argument and char16ptr_t is not a real pointer on GCC. In this case what we really want is const_cast, but that requires argument to be exactly the expected type, so this would be something like: const_cast<wchar_t*>(static_cast<const wchar_t*>(aPrintName.get())) That's why I decided that C-style cast looks better in this case, but I will change it if you like.
Comment 3•11 years ago
|
||
(In reply to comment #2) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #1) > > Comment on attachment 8341075 [details] [diff] [review] > > patch.diff > > > > Review of attachment 8341075 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: embedding/components/printingui/src/win/nsPrintDialogUtil.cpp > > @@ +687,5 @@ > > > HGLOBAL hGlobalDevMode = nullptr; > > > > > > HANDLE hPrinter = nullptr; > > > // const cast kludge for silly Win32 api's > > > + LPWSTR printName = (LPWSTR)aPrintName.get(); > > > > Please use reinterpret_cast here. > > The problem here is that reinterpret_cast requires pointer argument and > char16ptr_t is not a real pointer on GCC. In this case what we really want is > const_cast, but that requires argument to be exactly the expected type, so this > would be something like: > const_cast<wchar_t*>(static_cast<const wchar_t*>(aPrintName.get())) > That's why I decided that C-style cast looks better in this case, but I will > change it if you like. I see. The explicit casts are probably a bit clearer, so it would be great if you could switch to that please.
Assignee | ||
Comment 4•11 years ago
|
||
OK, I changed that. Thanks for review. https://hg.mozilla.org/integration/mozilla-inbound/rev/32fcd54fd18b
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/32fcd54fd18b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
status-firefox28:
--- → fixed
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•