Mercurial > hg
changeset 51687:1eab9e40c0c8
convert: fix various leaked file descriptors
Some of these only leaked if an exception occurred between the open and close,
but a lot of these leaked unconditionally.
A type hint is added to `parsesplicemap` because otherwise this change caused
pytype to change the return type from this to `Dict[nothing, nothing]`.
| author | Matt Harbison <matt_harbison@yahoo.com> |
|---|---|
| date | Thu, 11 Jul 2024 21:54:02 -0400 |
| parents | 39033e7a6e0a |
| children | 25e7f9dcad0f |
| files | hgext/convert/common.py hgext/convert/convcmd.py hgext/convert/cvs.py hgext/convert/cvsps.py hgext/convert/monotone.py hgext/convert/subversion.py |
| diffstat | 6 files changed, 77 insertions(+), 74 deletions(-) [+] |
line wrap: on
line diff
--- a/hgext/convert/common.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/common.py Thu Jul 11 21:54:02 2024 -0400 @@ -575,22 +575,25 @@ fp = open(self.path, b'rb') except FileNotFoundError: return - for i, line in enumerate(fp): - line = line.splitlines()[0].rstrip() - if not line: - # Ignore blank lines - continue - try: - key, value = line.rsplit(b' ', 1) - except ValueError: - raise error.Abort( - _(b'syntax error in %s(%d): key/value pair expected') - % (self.path, i + 1) - ) - if key not in self: - self.order.append(key) - super(mapfile, self).__setitem__(key, value) - fp.close() + + try: + for i, line in enumerate(fp): + line = line.splitlines()[0].rstrip() + if not line: + # Ignore blank lines + continue + try: + key, value = line.rsplit(b' ', 1) + except ValueError: + raise error.Abort( + _(b'syntax error in %s(%d): key/value pair expected') + % (self.path, i + 1) + ) + if key not in self: + self.order.append(key) + super(mapfile, self).__setitem__(key, value) + finally: + fp.close() def __setitem__(self, key, value) -> None: if self.fp is None:
--- a/hgext/convert/convcmd.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/convcmd.py Thu Jul 11 21:54:02 2024 -0400 @@ -13,6 +13,8 @@ from typing import ( AnyStr, + Dict, + List, Mapping, Optional, Union, @@ -297,7 +299,7 @@ self.splicemap = self.parsesplicemap(opts.get(b'splicemap')) self.branchmap = mapfile(ui, opts.get(b'branchmap')) - def parsesplicemap(self, path: bytes): + def parsesplicemap(self, path: bytes) -> Dict[bytes, List[bytes]]: """check and validate the splicemap format and return a child/parents dictionary. Format checking has two parts. @@ -312,31 +314,31 @@ return {} m = {} try: - fp = open(path, b'rb') - for i, line in enumerate(fp): - line = line.splitlines()[0].rstrip() - if not line: - # Ignore blank lines - continue - # split line - lex = common.shlexer(data=line, whitespace=b',') - line = list(lex) - # check number of parents - if not (2 <= len(line) <= 3): - raise error.Abort( - _( - b'syntax error in %s(%d): child parent1' - b'[,parent2] expected' + with open(path, b'rb') as fp: + for i, line in enumerate(fp): + line = line.splitlines()[0].rstrip() + if not line: + # Ignore blank lines + continue + # split line + lex = common.shlexer(data=line, whitespace=b',') + line = list(lex) + # check number of parents + if not (2 <= len(line) <= 3): + raise error.Abort( + _( + b'syntax error in %s(%d): child parent1' + b'[,parent2] expected' + ) + % (path, i + 1) ) - % (path, i + 1) - ) - for part in line: - self.source.checkrevformat(part) - child, p1, p2 = line[0], line[1:2], line[2:] - if p1 == p2: - m[child] = p1 - else: - m[child] = p1 + p2 + for part in line: + self.source.checkrevformat(part) + child, p1, p2 = line[0], line[1:2], line[2:] + if p1 == p2: + m[child] = p1 + else: + m[child] = p1 + p2 # if file does not exist or error reading, exit except IOError: raise error.Abort( @@ -509,14 +511,13 @@ authorfile = self.authorfile if authorfile: self.ui.status(_(b'writing author map file %s\n') % authorfile) - ofile = open(authorfile, b'wb+') - for author in self.authors: - ofile.write( - util.tonativeeol( - b"%s=%s\n" % (author, self.authors[author]) + with open(authorfile, b'wb+') as ofile: + for author in self.authors: + ofile.write( + util.tonativeeol( + b"%s=%s\n" % (author, self.authors[author]) + ) ) - ) - ofile.close() def readauthormap(self, authorfile) -> None: self.authors = readauthormap(self.ui, authorfile, self.authors)
--- a/hgext/convert/cvs.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/cvs.py Thu Jul 11 21:54:02 2024 -0400 @@ -11,9 +11,6 @@ import socket from mercurial.i18n import _ -from mercurial.pycompat import ( - open, -) from mercurial import ( encoding, error, @@ -52,8 +49,8 @@ self.tags = {} self.lastbranch = {} self.socket = None - self.cvsroot = open(os.path.join(cvs, b"Root"), b'rb').read()[:-1] - self.cvsrepo = open(os.path.join(cvs, b"Repository"), b'rb').read()[:-1] + self.cvsroot = util.readfile(os.path.join(cvs, b"Root"))[:-1] + self.cvsrepo = util.readfile(os.path.join(cvs, b"Repository"))[:-1] self.encoding = encoding.encoding self._connect() @@ -160,8 +157,7 @@ passw = b"A" cvspass = os.path.expanduser(b"~/.cvspass") try: - pf = open(cvspass, b'rb') - for line in pf.read().splitlines(): + for line in util.readfile(cvspass).splitlines(): part1, part2 = line.split(b' ', 1) # /1 :pserver:user@example.com:2401/cvsroot/foo # Ah<Z @@ -174,7 +170,6 @@ if part1 == format: passw = part2 break - pf.close() except IOError as inst: if inst.errno != errno.ENOENT: if not getattr(inst, 'filename', None):
--- a/hgext/convert/cvsps.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/cvsps.py Thu Jul 11 21:54:02 2024 -0400 @@ -161,7 +161,7 @@ # Use the Root file in the sandbox, if it exists try: - root = open(os.path.join(b'CVS', b'Root'), b'rb').read().strip() + root = util.readfile(os.path.join(b'CVS', b'Root')).strip() except IOError: pass @@ -195,16 +195,17 @@ if cache == b'update': try: ui.note(_(b'reading cvs log cache %s\n') % cachefile) - oldlog = pickle.load(open(cachefile, b'rb')) - for e in oldlog: - if not ( - hasattr(e, 'branchpoints') - and hasattr(e, 'commitid') - and hasattr(e, 'mergepoint') - ): - ui.status(_(b'ignoring old cache\n')) - oldlog = [] - break + with open(cachefile, b'rb') as fp: + oldlog = pickle.load(fp) + for e in oldlog: + if not ( + hasattr(e, 'branchpoints') + and hasattr(e, 'commitid') + and hasattr(e, 'mergepoint') + ): + ui.status(_(b'ignoring old cache\n')) + oldlog = [] + break ui.note(_(b'cache has %d log entries\n') % len(oldlog)) except Exception as e: @@ -526,7 +527,9 @@ # write the new cachefile ui.note(_(b'writing cvs log cache %s\n') % cachefile) - pickle.dump(log, open(cachefile, b'wb')) + + with open(cachefile, b'wb') as fp: + pickle.dump(log, fp) else: log = oldlog
--- a/hgext/convert/monotone.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/monotone.py Thu Jul 11 21:54:02 2024 -0400 @@ -43,9 +43,8 @@ if not os.path.exists(os.path.join(path, b'_MTN')): # Could be a monotone repository (SQLite db file) try: - f = open(path, b'rb') - header = f.read(16) - f.close() + with open(path, b'rb') as f: + header = f.read(16) except IOError: header = b'' if header != b'SQLite format 3\x00':
--- a/hgext/convert/subversion.py Thu Jul 11 21:16:45 2024 -0400 +++ b/hgext/convert/subversion.py Thu Jul 11 21:54:02 2024 -0400 @@ -1488,9 +1488,11 @@ prop_actions_allowed.append((b'M', b'svn:date')) hook = os.path.join(created, b'hooks', b'pre-revprop-change') - fp = open(hook, b'wb') - fp.write(gen_pre_revprop_change_hook(prop_actions_allowed)) - fp.close() + + util.writefile( + hook, gen_pre_revprop_change_hook(prop_actions_allowed) + ) + util.setflags(hook, False, True) output = self.run0(b'info')