git: 6687410af7db - main - makesyscalls: sprinkle some assert() on standard function calls

[Kyle Evans]

The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=6687410af7dba54e19b773684a969e9c590999e4

commit 6687410af7dba54e19b773684a969e9c590999e4
Author:     Kyle Evans <kevans at FreeBSD.org>
AuthorDate: 2021-01-27 18:12:33 +0000
Commit:     Kyle Evans <kevans at FreeBSD.org>
CommitDate: 2021-09-25 01:55:56 +0000

    makesyscalls: sprinkle some assert() on standard function calls
    
    Improves our error reporting, ensuring that we aren't just ignoring
    errors in the common case.
    
    Note specifically the boundary where we have to change up our error
    handling approach.  It's fine to error() out up until we create the
    tempdir, then the rest should try to handle it gracefully and abort().
    A future change will clean this up further by pcall'ing all of the bits
    that cannot currently error() without cleaning up.
---
 sys/tools/makesyscalls.lua | 64 ++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/sys/tools/makesyscalls.lua b/sys/tools/makesyscalls.lua
index 64b260eb65bf..7824a533e4d5 100644
--- a/sys/tools/makesyscalls.lua
+++ b/sys/tools/makesyscalls.lua
@@ -95,28 +95,30 @@ local files = {}
 
 local function cleanup()
 	for _, v in pairs(files) do
-		v:close()
+		assert(v:close())
 	end
 	if cleantmp then
 		if lfs.dir(tmpspace) then
 			for fname in lfs.dir(tmpspace) do
 				if fname ~= "." and fname ~= ".." then
-					os.remove(tmpspace .. "/" .. fname))
+					assert(os.remove(tmpspace .. "/" ..
+					    fname))
 				end
 			end
 		end
 
 		if lfs.attributes(tmpspace) and not lfs.rmdir(tmpspace) then
-			io.stderr:write("Failed to clean up tmpdir: " ..
-			    tmpspace .. "\n")
+			assert(io.stderr:write("Failed to clean up tmpdir: " ..
+			    tmpspace .. "\n"))
 		end
 	else
-		io.stderr:write("Temp files left in " .. tmpspace .. "\n")
+		assert(io.stderr:write("Temp files left in " .. tmpspace ..
+		    "\n"))
 	end
 end
 
 local function abort(status, msg)
-	io.stderr:write(msg .. "\n")
+	assert(io.stderr:write(msg .. "\n"))
 	cleanup()
 	os.exit(status)
 end
@@ -208,14 +210,11 @@ local function process_config(file)
 	-- would need to sanitize the line for potentially special characters.
 	local line_expr = "^([%w%p]+%s*)=(%s*[`\"]?[^\"`]+[`\"]?)"
 
-	if file == nil then
+	if not file then
 		return nil, "No file given"
 	end
 
-	local fh = io.open(file)
-	if fh == nil then
-		return nil, "Could not open file"
-	end
+	local fh = assert(io.open(file))
 
 	for nextline in fh:lines() do
 		-- Strip any whole-line comments
@@ -262,7 +261,7 @@ local function process_config(file)
 		end
 	end
 
-	io.close(fh)
+	assert(io.close(fh))
 	return cfg
 end
 
@@ -291,7 +290,7 @@ local function grab_capenabled(file, open_fail_ok)
 		end
 	end
 
-	io.close(fh)
+	assert(io.close(fh))
 	return capentries
 end
 
@@ -369,8 +368,8 @@ local function read_file(tmpfile)
 	end
 
 	local fh = files[tmpfile]
-	fh:seek("set")
-	return fh:read("a")
+	assert(fh:seek("set"))
+	return assert(fh:read("a"))
 end
 
 local function write_line(tmpfile, line)
@@ -378,13 +377,13 @@ local function write_line(tmpfile, line)
 		print("Not found: " .. tmpfile)
 		return
 	end
-	files[tmpfile]:write(line)
+	assert(files[tmpfile]:write(line))
 end
 
 local function write_line_pfile(tmppat, line)
 	for k in pairs(files) do
 		if k:match(tmppat) ~= nil then
-			files[k]:write(line)
+			assert(files[k]:write(line))
 		end
 	end
 end
@@ -505,7 +504,7 @@ local function process_sysfile(file)
 		process_syscall_def(prevline)
 	end
 
-	io.close(fh)
+	assert(io.close(fh))
 	return capentries
 end
 
@@ -1108,7 +1107,7 @@ end
 -- Entry point
 
 if #arg < 1 or #arg > 2 then
-	abort(1, "usage: " .. arg[0] .. " input-file <config-file>")
+	error("usage: " .. arg[0] .. " input-file <config-file>")
 end
 
 local sysfile, configfile = arg[1], arg[2]
@@ -1116,13 +1115,7 @@ local sysfile, configfile = arg[1], arg[2]
 -- process_config either returns nil and a message, or a
 -- table that we should merge into the global config
 if configfile ~= nil then
-	local res, msg = process_config(configfile)
-
-	if res == nil then
-		-- Error... handle?
-		print(msg)
-		os.exit(1)
-	end
+	local res = assert(process_config(configfile))
 
 	for k, v in pairs(res) do
 		if v ~= config[k] then
@@ -1150,17 +1143,28 @@ process_compat()
 process_abi_flags()
 
 if not lfs.mkdir(tmpspace) then
-	abort(1, "Failed to create tempdir " .. tmpspace)
+	error("Failed to create tempdir " .. tmpspace)
 end
 
+-- XXX Revisit the error handling here, we should probably move the rest of this
+-- into a function that we pcall() so we can catch the errors and clean up
+-- gracefully.
 for _, v in ipairs(temp_files) do
 	local tmpname = tmpspace .. v
 	files[v] = io.open(tmpname, "w+")
+	-- XXX Revisit these with a pcall() + error handler
+	if not files[v] then
+		abort(1, "Failed to open temp file: " .. tmpname)
+	end
 end
 
 for _, v in ipairs(output_files) do
 	local tmpname = tmpspace .. v
 	files[v] = io.open(tmpname, "w+")
+	-- XXX Revisit these with a pcall() + error handler
+	if not files[v] then
+		abort(1, "Failed to open temp output file: " .. tmpname)
+	end
 end
 
 -- Write out all of the preamble bits
@@ -1358,12 +1362,12 @@ write_line("systrace", read_file("systraceret"))
 for _, v in ipairs(output_files) do
 	local target = config[v]
 	if target ~= "/dev/null" then
-		local fh = io.open(target, "w+")
+		local fh = assert(io.open(target, "w+"))
 		if fh == nil then
 			abort(1, "Failed to open '" .. target .. "'")
 		end
-		fh:write(read_file(v))
-		fh:close()
+		assert(fh:write(read_file(v)))
+		assert(fh:close())
 	end
 end