[PATCH] Use try/finally to close files on error
Victor Stinner
victor.stinner at haypocalc.com
Wed Nov 2 15:42:25 CDT 2011
Hi,
Following patch close the open file in ignore.ignore(),
revlog.revlog._loadchunk(), ui.ui.readconfig(). It uses also "try: ...
finally: fp.close()" pattern in other places to ensure that the file
is also closed on error.
Victor
--
exporting patch:
# HG changeset patch
# User Victor Stinner <victor.stinner at haypocalc.com>
# Date 1320266504 -3600
# Node ID d49fd76a1ec1e317e42d63ed75d006bb02fb0115
# Parent 872f06c342ffa01f414cb57506090faaff410cd4
Close the file in ignore.ignore(), revlog.revlog._loadchunk(),
ui.ui.readconfig().
Use "try: ... finally: fp.close()" in other places to ensure that the
file is
also closed on error.
diff --git a/mercurial/changelog.py b/mercurial/changelog.py
--- a/mercurial/changelog.py
+++ b/mercurial/changelog.py
@@ -130,8 +130,10 @@ class changelog(revlog.revlog):
util.rename(n, n[:-2])
elif self._delaybuf:
fp = self.opener(self.indexfile, 'a')
- fp.write("".join(self._delaybuf))
- fp.close()
+ try:
+ fp.write("".join(self._delaybuf))
+ finally:
+ fp.close()
self._delaybuf = []
# split when we're done
self.checkinlinesize(tr)
diff --git a/mercurial/ignore.py b/mercurial/ignore.py
--- a/mercurial/ignore.py
+++ b/mercurial/ignore.py
@@ -77,7 +77,10 @@ def ignore(root, files, warn):
try:
pats[f] = []
fp = open(f)
- pats[f], warnings = ignorepats(fp)
+ try:
+ pats[f], warnings = ignorepats(fp)
+ finally:
+ fp.close()
for warning in warnings:
warn("%s: %s\n" % (f, warning))
except IOError, inst:
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -118,9 +118,11 @@ class localrepository(repo.repository):
def _writerequirements(self):
reqfile = self.opener("requires", "w")
- for r in self.requirements:
- reqfile.write("%s\n" % r)
- reqfile.close()
+ try:
+ for r in self.requirements:
+ reqfile.write("%s\n" % r)
+ finally:
+ reqfile.close()
def _checknested(self, path):
"""Determine if path is a legal nested repository."""
@@ -267,7 +269,6 @@ class localrepository(repo.repository):
old = self.tags().get(name, nullid)
fp.write('%s %s\n' % (hex(old), m))
fp.write('%s %s\n' % (hex(node), m))
- fp.close()
prevtags = ''
if local:
@@ -278,8 +279,11 @@ class localrepository(repo.repository):
else:
prevtags = fp.read()
- # local tags are stored in the current charset
- writetags(fp, names, None, prevtags)
+ try:
+ # local tags are stored in the current charset
+ writetags(fp, names, None, prevtags)
+ finally:
+ fp.close()
for name in names:
self.hook('tag', node=hex(node), tag=name, local=local)
return
@@ -293,10 +297,11 @@ class localrepository(repo.repository):
else:
prevtags = fp.read()
- # committed tags are stored in UTF-8
- writetags(fp, names, encoding.fromlocal, prevtags)
-
- fp.close()
+ try:
+ # committed tags are stored in UTF-8
+ writetags(fp, names, encoding.fromlocal, prevtags)
+ finally:
+ fp.close()
if '.hgtags' not in self.dirstate:
self[None].add(['.hgtags'])
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -238,8 +238,10 @@ class revlog(object):
self._initempty = True
try:
f = self.opener(self.indexfile)
- i = f.read()
- f.close()
+ try:
+ i = f.read()
+ finally:
+ f.close()
if len(i) > 0:
v = struct.unpack(versionformat, i[:4])[0]
self._initempty = False
@@ -792,14 +794,17 @@ class revlog(object):
self._chunkcache = offset, data
def _loadchunk(self, offset, length):
+ readahead = max(65536, length)
+
if self._inline:
df = self.opener(self.indexfile)
else:
df = self.opener(self.datafile)
-
- readahead = max(65536, length)
- df.seek(offset)
- d = df.read(readahead)
+ try:
+ df.seek(offset)
+ d = df.read(readahead)
+ finally:
+ df.close()
self._addchunk(offset, d)
if readahead > length:
return d[:length]
@@ -940,15 +945,16 @@ class revlog(object):
df.close()
fp = self.opener(self.indexfile, 'w', atomictemp=True)
- self.version &= ~(REVLOGNGINLINEDATA)
- self._inline = False
- for i in self:
- e = self._io.packentry(self.index[i], self.node,
self.version, i)
- fp.write(e)
-
- # if we don't call close, the temp file will never replace the
- # real index
- fp.close()
+ try:
+ self.version &= ~(REVLOGNGINLINEDATA)
+ self._inline = False
+ for i in self:
+ e = self._io.packentry(self.index[i], self.node,
self.version, i)
+ fp.write(e)
+ finally:
+ # if we don't call close, the temp file will never replace the
+ # real index
+ fp.close()
tr.replace(self.indexfile, trindex * self._io.size)
self._chunkclear()
diff --git a/mercurial/store.py b/mercurial/store.py
--- a/mercurial/store.py
+++ b/mercurial/store.py
@@ -324,12 +324,14 @@ class fncache(object):
except IOError:
# skip nonexistent file
return
- for n, line in enumerate(fp):
- if (len(line) < 2) or (line[-1] != '\n'):
- t = _('invalid entry in fncache, line %s') % (n + 1)
- raise util.Abort(t)
- self.entries.add(decodedir(line[:-1]))
- fp.close()
+ try:
+ for n, line in enumerate(fp):
+ if (len(line) < 2) or (line[-1] != '\n'):
+ t = _('invalid entry in fncache, line %s') % (n + 1)
+ raise util.Abort(t)
+ self.entries.add(decodedir(line[:-1]))
+ finally:
+ fp.close()
def rewrite(self, files):
fp = self.opener('fncache', mode='wb')
@@ -343,9 +345,11 @@ class fncache(object):
if not self._dirty:
return
fp = self.opener('fncache', mode='wb', atomictemp=True)
- for p in self.entries:
- fp.write(encodedir(p) + '\n')
- fp.close()
+ try:
+ for p in self.entries:
+ fp.write(encodedir(p) + '\n')
+ finally:
+ fp.close()
self._dirty = False
def add(self, fn):
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -373,19 +373,21 @@ class hgsubrepo(abstractsubrepo):
if create:
fp = self._repo.opener("hgrc", "w", text=True)
- fp.write('[paths]\n')
+ try:
+ fp.write('[paths]\n')
- def addpathconfig(key, value):
- if value:
- fp.write('%s = %s\n' % (key, value))
- self._repo.ui.setconfig('paths', key, value)
+ def addpathconfig(key, value):
+ if value:
+ fp.write('%s = %s\n' % (key, value))
+ self._repo.ui.setconfig('paths', key, value)
- defpath = _abssource(self._repo, abort=False)
- defpushpath = _abssource(self._repo, True, abort=False)
- addpathconfig('default', defpath)
- if defpath != defpushpath:
- addpathconfig('default-push', defpushpath)
- fp.close()
+ defpath = _abssource(self._repo, abort=False)
+ defpushpath = _abssource(self._repo, True, abort=False)
+ addpathconfig('default', defpath)
+ if defpath != defpushpath:
+ addpathconfig('default-push', defpushpath)
+ finally:
+ fp.close()
def add(self, ui, match, dryrun, prefix):
return cmdutil.add(ui, self._repo, match, dryrun, True,
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -74,38 +74,41 @@ class ui(object):
return
raise
- cfg = config.config()
- trusted = sections or trust or self._trusted(fp, filename)
+ try:
+ cfg = config.config()
+ trusted = sections or trust or self._trusted(fp, filename)
- try:
- cfg.read(filename, fp, sections=sections, remap=remap)
- except error.ConfigError, inst:
+ try:
+ cfg.read(filename, fp, sections=sections, remap=remap)
+ except error.ConfigError, inst:
+ if trusted:
+ raise
+ self.warn(_("Ignored: %s\n") % str(inst))
+
+ if self.plain():
+ for k in ('debug', 'fallbackencoding', 'quiet', 'slash',
+ 'logtemplate', 'style',
+ 'traceback', 'verbose'):
+ if k in cfg['ui']:
+ del cfg['ui'][k]
+ for k, v in cfg.items('defaults'):
+ del cfg['defaults'][k]
+ # Don't remove aliases from the configuration if in the
exceptionlist
+ if self.plain('alias'):
+ for k, v in cfg.items('alias'):
+ del cfg['alias'][k]
+
if trusted:
- raise
- self.warn(_("Ignored: %s\n") % str(inst))
+ self._tcfg.update(cfg)
+ self._tcfg.update(self._ocfg)
+ self._ucfg.update(cfg)
+ self._ucfg.update(self._ocfg)
- if self.plain():
- for k in ('debug', 'fallbackencoding', 'quiet', 'slash',
- 'logtemplate', 'style',
- 'traceback', 'verbose'):
- if k in cfg['ui']:
- del cfg['ui'][k]
- for k, v in cfg.items('defaults'):
- del cfg['defaults'][k]
- # Don't remove aliases from the configuration if in the
exceptionlist
- if self.plain('alias'):
- for k, v in cfg.items('alias'):
- del cfg['alias'][k]
-
- if trusted:
- self._tcfg.update(cfg)
- self._tcfg.update(self._ocfg)
- self._ucfg.update(cfg)
- self._ucfg.update(self._ocfg)
-
- if root is None:
- root = os.path.expanduser('~')
- self.fixconfig(root=root)
+ if root is None:
+ root = os.path.expanduser('~')
+ self.fixconfig(root=root)
+ finally:
+ fp.close()
def fixconfig(self, root=None, section=None):
if section in (None, 'paths'):
More information about the Mercurial-devel
mailing list