-
Notifications
You must be signed in to change notification settings - Fork 653
windows: Read the PATH env var of the child #1337
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -813,6 +813,20 @@ int make_program_env(char* env_block[], WCHAR** dst_ptr) { | |
return 0; | ||
} | ||
|
||
/* | ||
* Attempt to find the value of the PATH environment variable in the child's | ||
* preprocessed environment. | ||
* | ||
* If found, a pointer into `env` is returned. If not found, NULL is returned. | ||
*/ | ||
static WCHAR* find_path(WCHAR *env) { | ||
for (; env != NULL && *env != 0; env += wcslen(env)) { | ||
if (wcsncmp(env, L"PATH=", 5) == 0) | ||
return &env[5]; | ||
} | ||
|
||
return NULL; | ||
} | ||
|
||
/* | ||
* Called on Windows thread-pool thread to indicate that | ||
|
@@ -910,7 +924,7 @@ int uv_spawn(uv_loop_t* loop, | |
const uv_process_options_t* options) { | ||
int i; | ||
int err = 0; | ||
WCHAR* path = NULL; | ||
WCHAR* path = NULL, *alloc_path = NULL; | ||
BOOL result; | ||
WCHAR* application_path = NULL, *application = NULL, *arguments = NULL, | ||
*env = NULL, *cwd = NULL; | ||
|
@@ -984,7 +998,8 @@ int uv_spawn(uv_loop_t* loop, | |
} | ||
|
||
/* Get PATH environment variable. */ | ||
{ | ||
path = find_path(env); | ||
if (path == NULL) { | ||
DWORD path_len, r; | ||
|
||
path_len = GetEnvironmentVariableW(L"PATH", NULL, 0); | ||
|
@@ -993,11 +1008,12 @@ int uv_spawn(uv_loop_t* loop, | |
goto done; | ||
} | ||
|
||
path = (WCHAR*) malloc(path_len * sizeof(WCHAR)); | ||
if (path == NULL) { | ||
alloc_path = (WCHAR*) malloc(path_len * sizeof(WCHAR)); | ||
if (alloc_path == NULL) { | ||
err = ERROR_OUTOFMEMORY; | ||
goto done; | ||
} | ||
path = alloc_path; | ||
|
||
r = GetEnvironmentVariableW(L"PATH", path, path_len); | ||
if (r == 0 || r >= path_len) { | ||
|
@@ -1131,7 +1147,7 @@ int uv_spawn(uv_loop_t* loop, | |
free(arguments); | ||
free(cwd); | ||
free(env); | ||
free(path); | ||
free(alloc_path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this correct? We set path to alloc_path, so we should be freeing that one, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right. LGTM then. |
||
|
||
if (process->child_stdio_buffer != NULL) { | ||
/* Clean up child stdio handles. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1291,3 +1291,38 @@ TEST_IMPL(closed_fd_events) { | |
return 0; | ||
} | ||
#endif /* !_WIN32 */ | ||
|
||
TEST_IMPL(spawn_reads_child_path) { | ||
int r; | ||
int len; | ||
char path[1024]; | ||
char *env[2] = {path, NULL}; | ||
|
||
/* Set up the process, but make sure that the file to run is relative and */ | ||
/* requires a lookup into PATH */ | ||
init_process_options("spawn_helper1", exit_cb); | ||
options.file = "run-tests"; | ||
args[0] = "run-tests"; | ||
|
||
/* Set up the PATH env variable */ | ||
for (len = strlen(exepath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't len initialized twice here? |
||
exepath[len - 1] != '/' && exepath[len - 1] != '\\'; | ||
len--); | ||
exepath[len] = 0; | ||
strcpy(path, "PATH="); | ||
strcpy(path + 5, exepath); | ||
|
||
options.env = env; | ||
|
||
r = uv_spawn(uv_default_loop(), &process, &options); | ||
ASSERT(r == 0); | ||
|
||
r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); | ||
ASSERT(r == 0); | ||
|
||
ASSERT(exit_cb_called == 1); | ||
ASSERT(close_cb_called == 1); | ||
|
||
MAKE_VALGRIND_HAPPY(); | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you didn't introduce this, but can you remove the unnecessary cast? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for leaving the cast in :)