classification
Title: bug in new execvpe
Type: Stage:
Components: Library (Lib) Versions: Python 2.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: doko, gvanrossum, zackw
Priority: normal Keywords:

Created on 2002-08-28 01:03 by gvanrossum, last changed 2002-09-04 11:42 by gvanrossum. This issue is now closed.

Messages (6)
msg12177 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-08-28 01:03
From: "Steve M. Robbins" <steven.robbins@videotron.ca>


I think the patch associated with this thread has an
unintended
consequence.

In
http://mail.python.org/pipermail/python-dev/2002-August/027229.html
Zack pointed out three flaws in the original code:

    [...]
    Third, if an error other than the expected one
comes back, the
    loop clobbers the saved exception info and keeps
going.  Consider
    the situation where PATH=/bin:/usr/bin, /bin/foobar
exists but is
    not executable by the invoking user, and
/usr/bin/foobar does not
    exist.  The exception thrown will be 'No such file
or directory',
    not the expected 'Permission denied'.

The patch, as I understand it, changes the behaviour so
as to raise
the exception "Permission denied" in this case.

Consider a similar situation in which both /bin/foobar
(not executable
by the user) and /usr/bin/foobar (executable by the
user) exist.
Given the command "foobar", the shell will execute
/usr/bin/foobar.
If I understand the patch correctly, python will give
up when it
encounters /bin/foobar and raise the "Permission
denied" exception.

I believe this just happened to me today.  I had a
shell script named
"gcc" in ~/bin (first on my path) some months back. 
When I was
finished with it, I just did "chmod -x ~/bin/gcc" and
forgot about it.
Today was the first time since this patch went in that
I ran gcc via
python (using scipy's weave).  Boy was I surprised at
the message
"unable to execute gcc: Permission denied"!

I guess the fix is to save the EPERM exception and keep
going
in case there is an executable later in the path.
msg12178 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-08-28 01:06
Logged In: YES 
user_id=6380

Since I applied Zack's patch to the trunk as well as to the
2.2 maintenance branch, this needs to be fixed in both places.

This worked correctly in 2.1 and 2.2 (and in 2.2.1, but I
haven't tested that).
msg12179 - (view) Author: Zack Weinberg (zackw) Date: 2002-08-31 19:10
Logged In: YES 
user_id=580015

I've created a patch for this bug.  I would like to attach it,
but Sourceforge doesn't want to let me do that, so I'll just
insert it inline here:

===================================================================
Index: Lib/os.py
--- Lib/os.py	5 Aug 2002 16:13:24 -0000	1.59
+++ Lib/os.py	31 Aug 2002 19:08:34 -0000
@@ -309,7 +309,7 @@ def execvp(file, args):
     _execvpe(file, args)
 
 def execvpe(file, args, env):
-    """execv(file, args, env)
+    """execvpe(file, args, env)
 
     Execute the executable file (which is searched for
along $PATH)
     with argument list args and environment env , replacing the
@@ -339,14 +339,21 @@ def _execvpe(file, args, env=None):
     else:
         envpath = defpath
     PATH = envpath.split(pathsep)
+    saved_exc = None
+    saved_tb = None
     for dir in PATH:
         fullname = path.join(dir, file)
         try:
             apply(func, (fullname,) + argrest)
-        except error, (errno, msg):
-            if errno != ENOENT and errno != ENOTDIR:
-                raise
-    raise error, (errno, msg)
+        except error, e:
+            tb = sys.exc_info()[2]
+            if (e.errno != ENOENT and e.errno != ENOTDIR
+                and saved_exc is None):
+                saved_exc = e
+                saved_tb = tb
+    if saved_exc:
+        raise error, saved_exc, saved_tb
+    raise error, e, tb
 
 # Change environ to automatically call putenv() if it exists
 try:
msg12180 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-09-03 16:38
Logged In: YES 
user_id=6380

Thanks, Zack! Checked in for 2.2 and CVS head.
msg12181 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2002-09-04 06:58
Logged In: YES 
user_id=60903

should be checked in to the 2.1 branch as well.
msg12182 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-09-04 11:42
Logged In: YES 
user_id=6380

Good point, doko! Done.
History
Date User Action Args
2002-08-28 01:03:44gvanrossumcreate