Brad Lassey [:blassey] (use needinfo?) Reporter
	Description • 17 years ago

this doesn't include the changes in xpcom glue

	Benjamin Smedberg
	Comment 1 • 16 years ago

Comment on attachment 309242 [details] [diff] [review]
directy from "other" patch on bug 418703

>Index: xpcom/windbgdlg/windbgdlg.cpp

> int WINAPI 
> WinMain(HINSTANCE  hInstance, HINSTANCE  hPrevInstance, 
>         LPSTR  lpszCmdLine, int  nCmdShow)

>-    wsprintf(msg,
>-             "%s\n\nClick Abort to exit the Application.\n"
>-             "Click Retry to Debug the Application..\n"
>-             "Click Ignore to continue running the Application.", 
>+    wsprintfW(msg,
>+             L"%s\n\nClick Abort to exit the Application.\n"
>+             L"Click Retry to Debug the Application..\n"
>+             L"Click Ignore to continue running the Application.", 
>              lpszCmdLine);

You're passing lpszCmdLine as %s, which I think is incorrect for wsprintf... although I can't find a reference to wsprintfW docs. Can you clarify?

	Brian Crowder Assignee
	Comment 2 • 16 years ago

You are correct in asserting that %s is wrong for wsprintf(), which wants %S for narrow strings.  I have fixed that.  I also fixed a mixup/typo in nsDriveEnumerator (mLetter was swapped for mDrives).  I checked over the rest of the patch for spacing and sizeof/cast issues, but it's possible I've missed some, please double-check me again?

	Brian Crowder Assignee
	Comment 3 • 16 years ago

Comment on attachment 337982 [details] [diff] [review]
v2

Bah, I hate how MSVC's search leaves things highlighted, and therefore easily deleted/replaced.

	Brian Crowder Assignee
	Comment 5 • 16 years ago

Bernard:  I'd love to have you give this patch the same scrutiny as you've given others, if you have time.

	Bernard Alleysson
	Comment 6 • 16 years ago

Brian:

You changed:

+    wsprintfW(msg,
+              L"%S\n\nClick Abort to exit the Application.\n"
+              L"Click Retry to Debug the Application..\n"
+              L"Click Ignore to continue running the Application.", 
+              lpszCmdLine);

but lpszCmdLine is still ANSI, you need to use GetCommandLineW() here.

Please note that you are supposed to use LocalFree() to free the array returned by "CommandLineToArgvW".

	Brian Crowder Assignee
	Comment 7 • 16 years ago

%S is the right format-specifier for ANSI strings in wsprintf, according to MSDN

	Bernard Alleysson
	Comment 8 • 16 years ago

> %S is the right format-specifier for ANSI strings in wsprintf, according to
> MSDN

oops, I'm confused (I couldn't find the documentation). It should be ok then.

	Brian Crowder Assignee
	Comment 9 • 16 years ago

Added a LocalFree(argv)

	Benjamin Smedberg
	Comment 10 • 16 years ago

Comment on attachment 338144 [details] [diff] [review]
v4

r=me for everything *except* nsProcessCommon. The conversion in there currently 0-extends argv coming in, which is unacceptable.

	Brad Lassey [:blassey] (use needinfo?) Reporter
	Comment 11 • 16 years ago

crowder did you land this?

	Brian Crowder Assignee
	Comment 12 • 16 years ago

Nope, it needed another spin.