Opened 11 years ago
Closed 10 years ago
#287 closed defect (fixed)
Prioritize SHELL over EMXSHELL
Reported by: | dmik | Owned by: | bird |
---|---|---|---|
Priority: | normal | Milestone: | libc-0.6.6 |
Component: | libc | Version: | 0.6.5 |
Severity: | normal | Keywords: | |
Cc: |
Description
The EMXSHELL
is used to override COMSPEC
for popen()
and system()
calls in kLIBC. The order of checking in the first case is EMXSHELL, COMSPEC
and in the second — EMXSHELL, COMSPEC, OS2_SHELL, SHELL
.
At the same time, many scripts in the unix environment (e.g. configure scripts) use SHELL
to override the shell command to use. However, given that in the first case SHELL
isn't used at all and in the second case it's checked after EMXSHELL
and COMSPEC
, even if the script sets SHELL
(e.g. to point to sh.exe), kLIBC will not notice this and will still use the value of COMSPEC
(which is usually cmd.exe
). Not quite what the scripts expect.
I think it makes sense to check for SHELL
before checking for EMXSHELL
or COMSPEC
. The order should be the same in both calls: SHELL, EMXSHELL, COMSPEC, OS2_SHELL
.
The patch is trivial (in files lib/io/popen.c and lib/process/system.c) so not attaching it here.
Attachments (1)
Change History (10)
comment:2 by , 11 years ago
popen and system shouldn't consult any environment variables at all, it's hardcoded to /bin/sh normally. So, configure script don't expect system or popen to change behavior when they modify SHELL. Since we don't have any standard bourne shell on OS/2 by default, having EMXSHELL as pointer to it sounds good to me. Consulting SHELL before COMSPEC & OS2_SHELL would make sense to unixy processes, I guess.
I'll be changing the two calls to consider the same variables and in the same order: EMXSHELL, SHELL, COMSPEC, and OS2_SHELL. I'll also leave a todo for using /@unixroot/bin/sh[.exe] later, only this needs to only happen to wannabe-unix processes.
See also: http://pubs.opengroup.org/onlinepubs/009696699/functions/popen.html http://pubs.opengroup.org/onlinepubs/009696699/functions/system.html
comment:4 by , 11 years ago
Milestone: | libc-0.7 → libc-0.6.6 |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:7 by , 10 years ago
r3857 and r3859 introduced a regression. system() always falls into an interactive shell mode. An empty string test for pszCmdLine should be negated.
And if there is /@unixroot/bin/sh.exe, then system() cannot execute OS/2 builtin commands such as copy and del. How about using just env vars instead of checking a hardcoded path first ?
comment:8 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:9 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Thanks for noticing. Fixed in r3909.
BTW, if it's chanted as suggested, then http://trac.netlabs.org/rpm/log/python/trunk/Modules/posixmodule.c should also be changed accordingly (search for EMXSHELL there).