Closed Bug 422771 Opened 12 years ago Closed 11 years ago

reduce narrow windows API calls in browser

Categories

(Core :: General, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: blassey, Assigned: crowderbt)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attachment #309216 - Flags: review?(mconnor)
Comment on attachment 309216 [details] [diff] [review]
directy from "other" patch on bug 418703

delegating review to Jim so this gets done.
Attachment #309216 - Flags: review?(mconnor) → review?(jmathies)
>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)
Attachment #309216 - Flags: review?(jmathies) → review-
Assignee: nobody → crowder
Attachment #309216 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #332988 - Flags: review?
The CreateProcessW changes in nsWindowsShellService.cpp landed elsewhere already, somehow.  I didn't investigate.
Attachment #332988 - Flags: review? → review?(jmathies)
> 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. 
Attachment #332988 - Flags: review?(jmathies) → review+
> > 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!
http://hg.mozilla.org/mozilla-central/rev/b9de55a635d5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.