551375 - _PR_CreateWindowsProcess function is broken on WinCE

Status: RESOLVED WONTFIX

(NSPR :: NSPR, defect)

Description

[Alex Pakhotin (:alexp)]

CreateProcessW() API function on WinCE requires pszImageName parameter and the arguments in pszCmdLine.

Comment 1

[Alex Pakhotin (:alexp)]

Attachment: Fix

- assembleEnvBlock is not used for WinCE
- Pass the executable and arguments separately to CreateProcessW

Comment 2

[Alex Pakhotin (:alexp)]

Attachment: Fix v2

- Added path expansion using _wfullpath if a relative path to executable is provided (as WinCE does not support relative paths).
- Added a small fix for parent.c test, which verifies PR_CreateProcess() function.

Comment 3

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

Comment on attachment 431750 [details] [diff] [review]
Fix v2


>+    if (wideArgv0[0] == L'\\')
>+        wideExe = wideArgv0;
>+    else {
>+        /* Relative path is not supported by WinCE, so get an absolute path */
>+        wideAbsPath = PR_MALLOC(MAX_PATH * sizeof(PRUnichar));
>+        wideExe = _wfullpath(wideAbsPath, wideArgv0, MAX_PATH);
>+    }

I think this would be cleaner written like this:

if (wideArgv0[0] != L'\\' && wideArgv0[0] != L'/'){
    PRUnichar tmp = wideArgv0;
    wideArgv0 = PR_MALLOC(MAX_PATH * sizeof(PRUnichar));
    _wfullpath(wideArgv0, tmp, MAX_PATH);
    PR_FREE(tmp);
}


You can then drop wideAbsPath and wideExe and just use wideArgv0 for the rest of the function. Also, we accept both forward and back slashes in most of our path operations, no need to exclude forward slashes here.

everything else looks good, going to r- though because I'd like to see a patch with that change

Comment 4

[Alex Pakhotin (:alexp)]

Attachment: Fix v3

Agree with Brad - this way it's cleaner, and we can accept "/" too as CreateProcessW supports paths with forward slashes as well.

Also now executing the binary provided in the path parameter, not argv[0]. This makes more sense and seems to be more consistent with implementation for other systems.
Wan-Teh, what do you think about this bit?

Comment 5

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

Comment on attachment 431972 [details] [diff] [review]
Fix v3

looks good, but I defer to Wan-Teh on the path/argv[0] bit