[PATCHv4,7/7] zdtm: Explicitly close opened files

Submitted by Radostin Stoyanov on Sept. 13, 2018, 5:28 p.m.

Details

Message ID 20180913172811.4332-8-rstoyanov1@gmail.com
State Accepted
Series "remote: Refactor TCP client/server setup"
Headers show

Commit Message

Radostin Stoyanov Sept. 13, 2018, 5:28 p.m.
CPython currently uses a reference-counting scheme with (optional)
delayed detection of cyclically linked garbage, which collects most
objects as soon as they become unreachable, but is not guaranteed to
collect garbage containing circular references.

Some objects contain references to "external" resources such as open
files. It is understood that these resources are freed when the object
is garbage-collected, but since garbage collection is not guaranteed to
happen, such objects also provide an explicit way to release the
external resource, usually a close() method.

Programs are strongly recommended to explicitly close such objects.

Reference: https://docs.python.org/3.6/reference/datamodel.html

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 test/zdtm.py | 100 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 41 deletions(-)

Patch hide | download patch | download mbox

diff --git a/test/zdtm.py b/test/zdtm.py
index 6a89ee25..eb9fe6a0 100755
--- a/test/zdtm.py
+++ b/test/zdtm.py
@@ -108,13 +108,10 @@  def add_to_output(path):
 	if not report_dir:
 		return
 
-	fdi = open(path, "r")
-	fdo = open(os.path.join(report_dir, "output"), "a")
-	while True:
-		buf = fdi.read(1 << 20)
-		if not buf:
-			break
-		fdo.write(buf)
+	output_path = os.path.join(report_dir, "output")
+	with open(path, "r") as fdi, open(output_path, "a") as fdo:
+		for line in fdi:
+			fdo.write(line)
 
 
 prev_crash_reports = set(glob.glob("/tmp/zdtm-core-*.txt"))
@@ -131,7 +128,8 @@  def check_core_files():
 	for i in reports:
 		add_to_report(i, os.path.basename(i))
 		print_sep(i)
-		print(open(i).read())
+		with open(i, "r") as report:
+			print(report.read())
 		print_sep(i)
 
 	return True
@@ -318,7 +316,8 @@  def tail(path):
 
 
 def rpidfile(path):
-	return open(path).readline().strip()
+	with open(path) as fd:
+		return fd.readline().strip()
 
 
 def wait_pid_die(pid, who, tmo = 30):
@@ -502,7 +501,8 @@  class zdtm_test:
 		if 'PASS' not in list(map(lambda s: s.strip(), res.split())):
 			if os.access(self.__name + '.out.inprogress', os.F_OK):
 				print_sep(self.__name + '.out.inprogress')
-				print(open(self.__name + '.out.inprogress').read())
+				with open(self.__name + '.out.inprogress') as fd:
+					print(fd.read())
 				print_sep(self.__name + '.out.inprogress')
 			raise test_fail_exc("result check")
 
@@ -547,7 +547,8 @@  class zdtm_test:
 	def print_output(self):
 		if os.access(self.__name + '.out', os.R_OK):
 			print("Test output: " + "=" * 32)
-			print(open(self.__name + '.out').read())
+			with open(self.__name + '.out') as output:
+				print(output.read())
 			print(" <<< " + "=" * 32)
 
 	def static(self):
@@ -634,7 +635,8 @@  class inhfd_test:
 		self.__my_file.write(self.__message)
 		self.__my_file.flush()
 		pid, status = os.waitpid(self.__peer_pid, 0)
-		print(open(self.__name + ".out").read())
+		with open(self.__name + ".out") as output:
+			print(output.read())
 		self.__peer_pid = 0
 		if not os.WIFEXITED(status) or os.WEXITSTATUS(status) != 42:
 			raise test_fail_exc("test failed with %d" % status)
@@ -688,7 +690,8 @@  class groups_test(zdtm_test):
 		zdtm_test.__init__(self, 'zdtm/lib/groups', desc, flavor, freezer)
 		if flavor.ns:
 			self.__real_name = name
-			self.__subs = map(lambda x: x.strip(), open(name).readlines())
+			with open(name) as fd:
+				self.__subs = map(lambda x: x.strip(), fd.readlines())
 			print("Subs:\n%s" % '\n'.join(self.__subs))
 		else:
 			self.__real_name = ''
@@ -861,7 +864,8 @@  class criu_rpc:
 				res = criu.restore()
 				pidf = ctx.get('pidf')
 				if pidf:
-					open(pidf, 'w').write('%d\n' % res.pid)
+					with open(pidf, 'w') as fd:
+						fd.write('%d\n' % res.pid)
 			elif action == "page-server":
 				res = criu.page_server_chld()
 				p = criu_rpc_process()
@@ -1006,7 +1010,8 @@  class criu:
 			fcntl.fcntl(fd, fcntl.F_SETFD, fdflags & ~fcntl.FD_CLOEXEC)
 			s_args += ["--status-fd", str(fd)]
 
-		ns_last_pid = open("/proc/sys/kernel/ns_last_pid").read()
+		with open("/proc/sys/kernel/ns_last_pid") as ns_last_pid_fd:
+			ns_last_pid = ns_last_pid_fd.read()
 
 		ret = self.__criu.run(action, s_args, self.__criu_bin, self.__fault, strace, preexec, nowait)
 
@@ -1032,7 +1037,8 @@  class criu:
 					os.rename(os.path.join(__ddir, log), os.path.join(__ddir, log + ".fail"))
 				# restore ns_last_pid to avoid a case when criu gets
 				# PID of one of restored processes.
-				open("/proc/sys/kernel/ns_last_pid", "w+").write(ns_last_pid)
+				with open("/proc/sys/kernel/ns_last_pid", "w+") as fd:
+					fd.write(ns_last_pid)
 				# try again without faults
 				print("Run criu " + action)
 				ret = self.__criu.run(action, s_args, self.__criu_bin, False, strace, preexec)
@@ -1301,7 +1307,8 @@  def get_visible_state(test):
 
 		cmaps = [[0, 0, ""]]
 		last = 0
-		for mp in open("/proc/%s/root/proc/%s/maps" % (test.getpid(), pid)):
+		mapsfd = open("/proc/%s/root/proc/%s/maps" % (test.getpid(), pid))
+		for mp in mapsfd:
 			m = list(map(lambda x: int('0x' + x, 0), mp.split()[0].split('-')))
 
 			m.append(mp.split()[1])
@@ -1316,14 +1323,16 @@  def get_visible_state(test):
 			else:
 				cmaps.append(m)
 				last += 1
+		mapsfd.close()
 
 		maps[pid] = set(map(lambda x: '%x-%x %s' % (x[0], x[1], " ".join(x[2:])), cmaps))
 
 		cmounts = []
 		try:
 			r = re.compile("^\S+\s\S+\s\S+\s(\S+)\s(\S+)\s\S+\s[^-]*?(shared)?[^-]*?(master)?[^-]*?-")
-			for m in open("/proc/%s/root/proc/%s/mountinfo" % (test.getpid(), pid)):
-				cmounts.append(r.match(m).groups())
+			with open("/proc/%s/root/proc/%s/mountinfo" % (test.getpid(), pid)) as mountinfo:
+				for m in mountinfo:
+					cmounts.append(r.match(m).groups())
 		except IOError as e:
 			if e.errno != errno.EINVAL:
 				raise e
@@ -1614,7 +1623,8 @@  class Launcher:
 			print(u"# Timestamp: " + now.strftime("%Y-%m-%d %H:%M") + " (GMT+1)", file=self.__file_report)
 			print(u"# ", file=self.__file_report)
 			print(u"1.." + str(nr_tests), file=self.__file_report)
-		self.__taint = open("/proc/sys/kernel/tainted").read()
+		with open("/proc/sys/kernel/tainted") as taintfd:
+			self.__taint = taintfd.read()
 		if int(self.__taint, 0) != 0:
 			print("The kernel is tainted: %r" % self.__taint)
 			if not opts["ignore_taint"]:
@@ -1643,7 +1653,8 @@  class Launcher:
 		if len(self.__subs) >= self.__max:
 			self.wait()
 
-		taint = open("/proc/sys/kernel/tainted").read()
+		with open("/proc/sys/kernel/tainted") as taintfd:
+			taint = taintfd.read()
 		if self.__taint != taint:
 			raise Exception("The kernel is tainted: %r (%r)" % (taint, self.__taint))
 
@@ -1705,7 +1716,8 @@  class Launcher:
 				self.__failed.append([sub['name'], failed_flavor])
 				if self.__file_report:
 					testline = u"not ok %d - %s # flavor %s" % (self.__runtest, sub['name'], failed_flavor)
-					output = open(sub['log']).read()
+					with open(sub['log']) as sublog:
+						output = sublog.read()
 					details = {'output': output}
 					tc.add_error_info(output = output)
 					print(testline, file=self.__file_report)
@@ -1718,7 +1730,8 @@  class Launcher:
 					print(testline, file=self.__file_report)
 
 			if sub['log']:
-				print(open(sub['log']).read().encode('ascii', 'ignore'))
+				with open(sub['log']) as sublog:
+					print(sublog.read().encode('ascii', 'ignore'))
 				os.unlink(sub['log'])
 
 			return True
@@ -1749,6 +1762,7 @@  class Launcher:
 		if self.__file_report:
 			ts = TestSuite(opts['title'], self.__junit_test_cases, os.getenv("NODE_NAME"))
 			self.__junit_file.write(TestSuite.to_xml_string([ts]))
+			self.__junit_file.close()
 			self.__file_report.close()
 
 		if opts['keep_going']:
@@ -1767,7 +1781,8 @@  class Launcher:
 
 
 def all_tests(opts):
-	desc = eval(open(opts['set'] + '.desc').read())
+	with open(opts['set'] + '.desc') as fd:
+		desc = eval(fd.read())
 
 	files = []
 	mask = stat.S_IFREG | stat.S_IXUSR
@@ -1797,7 +1812,8 @@  default_test = {}
 def get_test_desc(tname):
 	d_path = tname + '.desc'
 	if os.access(d_path, os.F_OK) and os.path.getsize(d_path) > 0:
-		return eval(open(d_path).read())
+		with open(d_path) as fd:
+			return eval(fd.read())
 
 	return default_test
 
@@ -1831,22 +1847,23 @@  def grep_errors(fname):
 	first = True
 	print_next = False
 	before = []
-	for l in open(fname):
-		before.append(l)
-		if len(before) > 5:
-			before.pop(0)
-		if "Error" in l:
-			if first:
-				print_fname(fname, 'log')
-				print_sep("grep Error", "-", 60)
-				first = False
-			for i in before:
-				print_next = print_error(i)
-			before = []
-		else:
-			if print_next:
-				print_next = print_error(l)
+	with open(fname) as fd:
+		for l in fd:
+			before.append(l)
+			if len(before) > 5:
+				before.pop(0)
+			if "Error" in l:
+				if first:
+					print_fname(fname, 'log')
+					print_sep("grep Error", "-", 60)
+					first = False
+				for i in before:
+					print_next = print_error(i)
 				before = []
+			else:
+				if print_next:
+					print_next = print_error(l)
+					before = []
 	if not first:
 		print_sep("ERROR OVER", "-", 60)
 
@@ -1875,7 +1892,8 @@  def run_tests(opts):
 			print("No such file")
 			return
 
-		torun = map(lambda x: x.strip(), open(opts['from']))
+		with open(opts['from']) as fd:
+			torun = map(lambda x: x.strip(), fd)
 		opts['keep_going'] = False
 		run_all = True
 	else: