[gentoo-catalyst] [PATCH 05/13] lint: clean up bare exception handling - gentoo-catalyst

From:	Mike Frysinger <vapier@g.o>
To:	gentoo-catalyst@l.g.o
Subject:	[gentoo-catalyst] [PATCH 05/13] lint: clean up bare exception handling
Date:	Tue, 06 Oct 2015 15:05:51
Message-Id:	`1444143929-26705-5-git-send-email-vapier@gentoo.org`
In Reply to:	[gentoo-catalyst] [PATCH 01/13] lint: fix duplicate fcntl import by Mike Frysinger

1

It's a bad idea to use a bare except clause as you end up including

2

things like SystemExit, KeyboardInterrupt, and GeneratorExit, none

3

of which we actually want to catch.  Some of the cases in the code

4

were explicitly catching & passing SystemExit back up which proves

5

this point.

6

---

7

 catalyst/base/stagebase.py |  2 +-

8

 catalyst/lock.py           | 26 ++++----------------------

9

 catalyst/main.py           |  2 +-

10

 catalyst/support.py        | 15 +++++----------

11

 targets/stage1/build.py    |  2 +-

12

 5 files changed, 12 insertions(+), 35 deletions(-)

13

14

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py

15

index 409fcab..e393c5b 100644

16

--- a/catalyst/base/stagebase.py

17

+++ b/catalyst/base/stagebase.py

18

@@ -1015,7 +1015,7 @@ class StageBase(TargetBase, ClearBase, GenBase):

19

 					target is fully initialized

20

"""

21

 					self.snapshot_lock_object.unlock()

22

-				except:

23

+				except Exception:

24

 					pass

25

 		if ouch:

26

"""

27

diff --git a/catalyst/lock.py b/catalyst/lock.py

28

index 01b1aa8..d6653f7 100644

29

--- a/catalyst/lock.py

30

+++ b/catalyst/lock.py

31

@@ -130,8 +130,6 @@ class LockDir(object):

32

 				try:

33

 					if os.stat(self.lockfile).st_gid != self.gid:

34

 						os.chown(self.lockfile,os.getuid(),self.gid)

35

-				except SystemExit, e:

36

-					raise

37

 				except OSError, e:

38

 					if e[0] == 2: #XXX: No such file or directory

39

 						return self.fcntl_locking(locktype)

40

@@ -185,7 +183,7 @@ class LockDir(object):

41

 				try:

42

 					os.close(self.myfd)

43

 					self.myfd=None

44

-				except:

45

+				except Exception:

46

 					pass

47

 				return False

48

49

@@ -194,8 +192,6 @@ class LockDir(object):

50

 					self.myfd = os.open(self.lockfile, os.O_WRONLY,0660)

51

 					unlinkfile = 1

52

 					self.locking_method(self.myfd,fcntl.LOCK_UN)

53

-			except SystemExit, e:

54

-				raise e

55

 			except Exception, e:

56

 				#if self.myfd is not None:

57

 					#print "fcntl_unlock() trying to close", self.myfd

58

@@ -213,7 +209,7 @@ class LockDir(object):

59

 						InUse=False

60

 						try:

61

 							self.locking_method(self.myfd,fcntl.LOCK_EX|fcntl.LOCK_NB)

62

-						except:

63

+						except Exception:

64

 							print "Read lock may be in effect. skipping lockfile delete..."

65

 							InUse=True

66

 							# We won the lock, so there isn't competition for it.

67

@@ -227,8 +223,6 @@ class LockDir(object):

68

 						self.myfd=None

69

 #						if "DEBUG" in self.settings:

70

 #							print "Unlinked lockfile..."

71

-				except SystemExit, e:

72

-					raise e

73

 				except Exception, e:

74

 					# We really don't care... Someone else has the lock.

75

 					# So it is their problem now.

76

@@ -273,8 +267,6 @@ class LockDir(object):

77

 					print_traceback=True)

78

 			try:

79

 				os.link(self.myhardlock, self.lockfile)

80

-			except SystemExit:

81

-				raise

82

 			except Exception:

83

 #				if "DEBUG" in self.settings:

84

 #					print "lockfile(): Hardlink: Link failed."

85

@@ -305,9 +297,7 @@ class LockDir(object):

86

 				os.unlink(self.myhardlock)

87

 			if os.path.exists(self.lockfile):

88

 				os.unlink(self.lockfile)

89

-		except SystemExit:

90

-			raise

91

-		except:

92

+		except Exception:

93

 			writemsg("Something strange happened to our hardlink locks.\n")

94

95

 	def add_hardlock_file_to_cleanup(self):

96

@@ -335,9 +325,7 @@ class LockDir(object):

97

 		try:

98

 			myhls = os.stat(link)

99

 			mylfs = os.stat(lock)

100

-		except SystemExit:

101

-			raise

102

-		except:

103

+		except Exception:

104

 			myhls = None

105

 			mylfs = None

106

107

@@ -406,8 +394,6 @@ class LockDir(object):

108

 								# We're sweeping through, unlinking everyone's locks.

109

 								os.unlink(filename)

110

 								results.append("Unlinked: " + filename)

111

-							except SystemExit:

112

-								raise

113

 							except Exception:

114

 								pass

115

 					try:

116

@@ -415,16 +401,12 @@ class LockDir(object):

117

 						results.append("Unlinked: " + x)

118

 						os.unlink(mylockname)

119

 						results.append("Unlinked: " + mylockname)

120

-					except SystemExit:

121

-						raise

122

 					except Exception:

123

 						pass

124

 				else:

125

 					try:

126

 						os.unlink(mylockname)

127

 						results.append("Unlinked: " + mylockname)

128

-					except SystemExit:

129

-						raise

130

 					except Exception:

131

 						pass

132

 		return results

133

diff --git a/catalyst/main.py b/catalyst/main.py

134

index 04f689e..4e83414 100644

135

--- a/catalyst/main.py

136

+++ b/catalyst/main.py

137

@@ -95,7 +95,7 @@ def parse_config(myconfig):

138

 		myconfig = catalyst.config.ConfigParser(config_file)

139

 		myconf.update(myconfig.get_values())

140

141

-	except:

142

+	except Exception:

143

 		print "!!! catalyst: Unable to parse configuration file, "+myconfig

144

 		sys.exit(1)

145

146

diff --git a/catalyst/support.py b/catalyst/support.py

147

index 90c59eb..f184ed7 100644

148

--- a/catalyst/support.py

149

+++ b/catalyst/support.py

150

@@ -22,9 +22,7 @@ DESIRED_RLIMIT = 0

151

 try:

152

 	import resource

153

 	max_fd_limit=resource.getrlimit(resource.RLIMIT_NOFILE)[DESIRED_RLIMIT]

154

-except SystemExit, e:

155

-	raise

156

-except:

157

+except Exception:

158

 	# hokay, no resource module.

159

 	max_fd_limit=256

160

161

@@ -48,7 +46,7 @@ def read_from_clst(path):

162

 	myline = ''

163

 	try:

164

 		myf = open(path, "r")

165

-	except:

166

+	except Exception:

167

 		return -1

168

 		#raise CatalystError("Could not open file " + path)

169

 	for line in myf.readlines():

170

@@ -136,10 +134,7 @@ def cmd(mycmd, myexc="", env=None, debug=False, fail_func=None):

171

172

 	if debug:

173

 		print "***** cmd(); args =", args

174

-	try:

175

-		proc = Popen(args, env=env)

176

-	except:

177

-		raise

178

+	proc = Popen(args, env=env)

179

 	if proc.wait() != 0:

180

 		if fail_func:

181

 			print "CMD(), NON-Zero command return.  Running fail_func()"

182

@@ -243,7 +238,7 @@ def read_makeconf(mymakeconffile):

183

 				try:

184

 					import portage.util

185

 					return portage.util.getconfig(mymakeconffile, tolerant=1, allow_sourcing=True)

186

-				except:

187

+				except Exception:

188

 					try:

189

 						import portage_util

190

 						return portage_util.getconfig(mymakeconffile, tolerant=1, allow_sourcing=True)

191

@@ -252,7 +247,7 @@ def read_makeconf(mymakeconffile):

192

 						mylines=myf.readlines()

193

 						myf.close()

194

 						return parse_makeconf(mylines)

195

-		except:

196

+		except Exception:

197

 			raise CatalystError("Could not parse make.conf file " +

198

 				mymakeconffile, print_traceback=True)

199

 	else:

200

diff --git a/targets/stage1/build.py b/targets/stage1/build.py

201

index be1bc4d..fa4fd13 100755

202

--- a/targets/stage1/build.py

203

+++ b/targets/stage1/build.py

204

@@ -33,7 +33,7 @@ for idx in range(0, len(pkgs)):

205

 		buildpkgs[bidx] = pkgs[idx]

206

 		if buildpkgs[bidx][0:1] == "*":

207

 			buildpkgs[bidx] = buildpkgs[bidx][1:]

208

-	except:

209

+	except Exception:

210

 		pass

211

212

 for b in buildpkgs:

213

--

214

2.5.2

1	It's a bad idea to use a bare except clause as you end up including
2	things like SystemExit, KeyboardInterrupt, and GeneratorExit, none
3	of which we actually want to catch. Some of the cases in the code
4	were explicitly catching & passing SystemExit back up which proves
5	this point.
6	---
7	catalyst/base/stagebase.py \| 2 +-
8	catalyst/lock.py \| 26 ++++----------------------
9	catalyst/main.py \| 2 +-
10	catalyst/support.py \| 15 +++++----------
11	targets/stage1/build.py \| 2 +-
12	5 files changed, 12 insertions(+), 35 deletions(-)
13
14	diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
15	index 409fcab..e393c5b 100644
16	--- a/catalyst/base/stagebase.py
17	+++ b/catalyst/base/stagebase.py
18	@@ -1015,7 +1015,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
19	target is fully initialized
20	"""
21	self.snapshot_lock_object.unlock()
22	- except:
23	+ except Exception:
24	pass
25	if ouch:
26	"""
27	diff --git a/catalyst/lock.py b/catalyst/lock.py
28	index 01b1aa8..d6653f7 100644
29	--- a/catalyst/lock.py
30	+++ b/catalyst/lock.py
31	@@ -130,8 +130,6 @@ class LockDir(object):
32	try:
33	if os.stat(self.lockfile).st_gid != self.gid:
34	os.chown(self.lockfile,os.getuid(),self.gid)
35	- except SystemExit, e:
36	- raise
37	except OSError, e:
38	if e[0] == 2: #XXX: No such file or directory
39	return self.fcntl_locking(locktype)
40	@@ -185,7 +183,7 @@ class LockDir(object):
41	try:
42	os.close(self.myfd)
43	self.myfd=None
44	- except:
45	+ except Exception:
46	pass
47	return False
48
49	@@ -194,8 +192,6 @@ class LockDir(object):
50	self.myfd = os.open(self.lockfile, os.O_WRONLY,0660)
51	unlinkfile = 1
52	self.locking_method(self.myfd,fcntl.LOCK_UN)
53	- except SystemExit, e:
54	- raise e
55	except Exception, e:
56	#if self.myfd is not None:
57	#print "fcntl_unlock() trying to close", self.myfd
58	@@ -213,7 +209,7 @@ class LockDir(object):
59	InUse=False
60	try:
61	self.locking_method(self.myfd,fcntl.LOCK_EX\|fcntl.LOCK_NB)
62	- except:
63	+ except Exception:
64	print "Read lock may be in effect. skipping lockfile delete..."
65	InUse=True
66	# We won the lock, so there isn't competition for it.
67	@@ -227,8 +223,6 @@ class LockDir(object):
68	self.myfd=None
69	# if "DEBUG" in self.settings:
70	# print "Unlinked lockfile..."
71	- except SystemExit, e:
72	- raise e
73	except Exception, e:
74	# We really don't care... Someone else has the lock.
75	# So it is their problem now.
76	@@ -273,8 +267,6 @@ class LockDir(object):
77	print_traceback=True)
78	try:
79	os.link(self.myhardlock, self.lockfile)
80	- except SystemExit:
81	- raise
82	except Exception:
83	# if "DEBUG" in self.settings:
84	# print "lockfile(): Hardlink: Link failed."
85	@@ -305,9 +297,7 @@ class LockDir(object):
86	os.unlink(self.myhardlock)
87	if os.path.exists(self.lockfile):
88	os.unlink(self.lockfile)
89	- except SystemExit:
90	- raise
91	- except:
92	+ except Exception:
93	writemsg("Something strange happened to our hardlink locks.\n")
94
95	def add_hardlock_file_to_cleanup(self):
96	@@ -335,9 +325,7 @@ class LockDir(object):
97	try:
98	myhls = os.stat(link)
99	mylfs = os.stat(lock)
100	- except SystemExit:
101	- raise
102	- except:
103	+ except Exception:
104	myhls = None
105	mylfs = None
106
107	@@ -406,8 +394,6 @@ class LockDir(object):
108	# We're sweeping through, unlinking everyone's locks.
109	os.unlink(filename)
110	results.append("Unlinked: " + filename)
111	- except SystemExit:
112	- raise
113	except Exception:
114	pass
115	try:
116	@@ -415,16 +401,12 @@ class LockDir(object):
117	results.append("Unlinked: " + x)
118	os.unlink(mylockname)
119	results.append("Unlinked: " + mylockname)
120	- except SystemExit:
121	- raise
122	except Exception:
123	pass
124	else:
125	try:
126	os.unlink(mylockname)
127	results.append("Unlinked: " + mylockname)
128	- except SystemExit:
129	- raise
130	except Exception:
131	pass
132	return results
133	diff --git a/catalyst/main.py b/catalyst/main.py
134	index 04f689e..4e83414 100644
135	--- a/catalyst/main.py
136	+++ b/catalyst/main.py
137	@@ -95,7 +95,7 @@ def parse_config(myconfig):
138	myconfig = catalyst.config.ConfigParser(config_file)
139	myconf.update(myconfig.get_values())
140
141	- except:
142	+ except Exception:
143	print "!!! catalyst: Unable to parse configuration file, "+myconfig
144	sys.exit(1)
145
146	diff --git a/catalyst/support.py b/catalyst/support.py
147	index 90c59eb..f184ed7 100644
148	--- a/catalyst/support.py
149	+++ b/catalyst/support.py
150	@@ -22,9 +22,7 @@ DESIRED_RLIMIT = 0
151	try:
152	import resource
153	max_fd_limit=resource.getrlimit(resource.RLIMIT_NOFILE)[DESIRED_RLIMIT]
154	-except SystemExit, e:
155	- raise
156	-except:
157	+except Exception:
158	# hokay, no resource module.
159	max_fd_limit=256
160
161	@@ -48,7 +46,7 @@ def read_from_clst(path):
162	myline = ''
163	try:
164	myf = open(path, "r")
165	- except:
166	+ except Exception:
167	return -1
168	#raise CatalystError("Could not open file " + path)
169	for line in myf.readlines():
170	@@ -136,10 +134,7 @@ def cmd(mycmd, myexc="", env=None, debug=False, fail_func=None):
171
172	if debug:
173	print "***** cmd(); args =", args
174	- try:
175	- proc = Popen(args, env=env)
176	- except:
177	- raise
178	+ proc = Popen(args, env=env)
179	if proc.wait() != 0:
180	if fail_func:
181	print "CMD(), NON-Zero command return. Running fail_func()"
182	@@ -243,7 +238,7 @@ def read_makeconf(mymakeconffile):
183	try:
184	import portage.util
185	return portage.util.getconfig(mymakeconffile, tolerant=1, allow_sourcing=True)
186	- except:
187	+ except Exception:
188	try:
189	import portage_util
190	return portage_util.getconfig(mymakeconffile, tolerant=1, allow_sourcing=True)
191	@@ -252,7 +247,7 @@ def read_makeconf(mymakeconffile):
192	mylines=myf.readlines()
193	myf.close()
194	return parse_makeconf(mylines)
195	- except:
196	+ except Exception:
197	raise CatalystError("Could not parse make.conf file " +
198	mymakeconffile, print_traceback=True)
199	else:
200	diff --git a/targets/stage1/build.py b/targets/stage1/build.py
201	index be1bc4d..fa4fd13 100755
202	--- a/targets/stage1/build.py
203	+++ b/targets/stage1/build.py
204	@@ -33,7 +33,7 @@ for idx in range(0, len(pkgs)):
205	buildpkgs[bidx] = pkgs[idx]
206	if buildpkgs[bidx][0:1] == "*":
207	buildpkgs[bidx] = buildpkgs[bidx][1:]
208	- except:
209	+ except Exception:
210	pass
211
212	for b in buildpkgs:
213	--
214	2.5.2

Gentoo Archives: gentoo-catalyst