422771 - reduce narrow windows API calls in browser

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: directy from "other" patch on bug 418703

Comment 1

[Mike Connor [:mconnor]]

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

delegating review to Jim so this gets done.

Comment 2

[Jim Mathies [:jimm]]

>PRUnichar msg[2048]; 
>_vsnwprintf(msg, sizeof(msg), NS_ConvertUTF8toUTF16(fmt).get(), ap); 

sizeof(msg)?

>IUniformResourceLocatorW* urlLink = nsnull;

This is fine although if we have the time we might want to upgrade these to nsComPtr<>'s. 

>BOOL ok = CreateProcessW(NULL, (LPWSTR)appHelperPath.get(), NULL, NULL,

nit - (LPCWSTR)

Comment 3

[Brian Crowder]

Attachment: updated to match comments.

Comment 4

[Brian Crowder]

The CreateProcessW changes in nsWindowsShellService.cpp landed elsewhere already, somehow.  I didn't investigate.

Comment 5

[Jim Mathies [:jimm]]

> PRUnichar msg[2048]; 
>   _vsnwprintf(msg, sizeof(msg)/sizeof(msg[0]), NS_ConvertUTF8toUTF16(fmt).get
> (), ap); 

That works, an alternative would simply be 

sizeof(msg)/sizeof(PRUnichar) 

I guess msg[0] is actually more safe but I don't think it's a common convention. 

Comment 6

[Brian Crowder]

> > PRUnichar msg[2048]; 
> >   _vsnwprintf(msg, sizeof(msg)/sizeof(msg[0]), NS_ConvertUTF8toUTF16(fmt).get
> > (), ap); 
> 
> That works, an alternative would simply be 
> 
> sizeof(msg)/sizeof(PRUnichar) 
> 
> I guess msg[0] is actually more safe but I don't think it's a common
> convention.

I much prefer the style I used here, because if the type of "msg" ever changes in the future, this line of code will work correctly.  This pattern is common enough in the Mozilla codebase for there to be two macros supporting it (NS_ARRAY_LENGTH, and JS_ARRAY_LENGTH) -- I didn't use those here, because they aren't available, and I didn't want to introduce more coupling.  Thanks for the review!

Comment 7

[Brian Crowder]

http://hg.mozilla.org/mozilla-central/rev/b9de55a635d5