[PATCH] mkstemp: make sure we don't try to unlink files which were not created

Giorgos Keramidas keramida at ceid.upatras.gr
Wed May 2 08:02:47 CDT 2007


# HG changeset patch
# User Giorgos Keramidas <keramida at ceid.upatras.gr>
# Date 1178110924 -10800
# Node ID 52803b0fdf2e57f76b80ddc00c87809633b5b677
# Parent  15289406f89c3866e5a1e1ce7d666a7555a23631
mkstemp: make sure we don't try to unlink files which were not created

The tempfile.mkstemp() call may fail (i.e. because a file system is
mounted as readonly after some file system errors).  In this case, we
should not try to call os.unlink() or file.close() in finally: blocks.

Inspired by:	trace posted by 'mathieu' on #mercurial

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -19,11 +19,16 @@ def filemerge(repo, fw, fo, wctx, mctx):
 
     def temp(prefix, ctx):
         pre = "%s~%s." % (os.path.basename(ctx.path()), prefix)
-        (fd, name) = tempfile.mkstemp(prefix=pre)
-        data = repo.wwritedata(ctx.path(), ctx.data())
-        f = os.fdopen(fd, "wb")
-        f.write(data)
-        f.close()
+        f = None
+        fd = None
+        try:
+            (fd, name) = tempfile.mkstemp(prefix=pre)
+            data = repo.wwritedata(ctx.path(), ctx.data())
+            f = os.fdopen(fd, "wb")
+            f.write(data)
+        finally:
+            if f is not None:
+                f.close()
         return name
 
     fcm = wctx.filectx(fw)
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -45,9 +45,11 @@ def extract(ui, fileobj):
                         'retrieving revision [0-9]+(\.[0-9]+)*$|' +
                         '(---|\*\*\*)[ \t])', re.MULTILINE)
 
-    fd, tmpname = tempfile.mkstemp(prefix='hg-patch-')
-    tmpfp = os.fdopen(fd, 'w')
+    tmpfp = None
+    tmpname = None
     try:
+        fd, tmpname = tempfile.mkstemp(prefix='hg-patch-')
+        tmpfp = os.fdopen(fd, 'w')
         msg = email.Parser.Parser().parse(fileobj)
 
         message = msg['Subject']
@@ -116,8 +118,10 @@ def extract(ui, fileobj):
             elif not diffs_seen and message and content_type == 'text/plain':
                 message += '\n' + payload
     except:
-        tmpfp.close()
-        os.unlink(tmpname)
+        if tmpfp is not None:
+            tmpfp.close()
+        if tmpname is not None:
+            os.unlink(tmpname)
         raise
 
     tmpfp.close()
@@ -233,10 +237,12 @@ def dogitpatch(patchname, gitpatches, cw
     pf = file(patchname)
     pfline = 1
 
-    fd, patchname = tempfile.mkstemp(prefix='hg-patch-')
-    tmpfp = os.fdopen(fd, 'w')
-
+    tmpfp = None
+    patchname = None
     try:
+        fd, patchname = tempfile.mkstemp(prefix='hg-patch-')
+        tmpfp = os.fdopen(fd, 'w')
+
         for i in xrange(len(gitpatches)):
             p = gitpatches[i]
             if not p.copymod and not p.binary:
@@ -277,8 +283,10 @@ def dogitpatch(patchname, gitpatches, cw
             tmpfp.write(line)
             line = pf.readline()
     except:
-        tmpfp.close()
-        os.unlink(patchname)
+        if tmpfp is not None:
+            tmpfp.close()
+        if patchname is not None:
+            os.unlink(patchname)
         raise
 
     tmpfp.close()
@@ -637,8 +645,10 @@ def diffstat(patchlines):
 def diffstat(patchlines):
     if not util.find_in_path('diffstat', os.environ.get('PATH', '')):
         return
-    fd, name = tempfile.mkstemp(prefix="hg-patchbomb-", suffix=".txt")
+
+    name = None
     try:
+        fd, name = tempfile.mkstemp(prefix="hg-patchbomb-", suffix=".txt")
         p = popen2.Popen3('diffstat -p1 -w79 2>/dev/null > ' + name)
         try:
             for line in patchlines: print >> p.tochild, line
@@ -654,5 +664,5 @@ def diffstat(patchlines):
             return stat
         except: raise
     finally:
-        try: os.unlink(name)
-        except: pass
+        if name is not None:
+            os.unlink(name)
diff --git a/mercurial/sshserver.py b/mercurial/sshserver.py
--- a/mercurial/sshserver.py
+++ b/mercurial/sshserver.py
@@ -167,6 +167,7 @@ class sshserver(object):
 
         # write bundle data to temporary file because it can be big
 
+        fp, tempname = (None, None)
         try:
             fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
             fp = os.fdopen(fd, 'wb+')
@@ -197,8 +198,10 @@ class sshserver(object):
                     self.lock.release()
                     self.lock = None
         finally:
-            fp.close()
-            os.unlink(tempname)
+            if fp is not None:
+                fp.close()
+            if tempname is not None:
+                os.unlink(tempname)
 
     def do_stream_out(self):
         streamclone.stream_out(self.repo, self.fout)
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -417,9 +417,10 @@ class ui(object):
     def debug(self, *msg):
         if self.debugflag: self.write(*msg)
     def edit(self, text, user):
-        (fd, name) = tempfile.mkstemp(prefix="hg-editor-", suffix=".txt",
-                                      text=True)
+        name = None
         try:
+            (fd, name) = tempfile.mkstemp(prefix="hg-editor-",
+                                          suffix=".txt", text=True)
             f = os.fdopen(fd, "w")
             f.write(text)
             f.close()
@@ -437,7 +438,8 @@ class ui(object):
             f.close()
             t = re.sub("(?m)^HG:.*\n", "", t)
         finally:
-            os.unlink(name)
+            if name is not None:
+                os.unlink(name)
 
         return t
 


More information about the Mercurial-devel mailing list