Date: Sun, 3 Oct 2021 05:15:45 GMT From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 5129b3f90b50 - stable/13 - makesyscalls: sprinkle some assert() on standard function calls Message-ID: <202110030515.1935Fjpo008829@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=5129b3f90b5069236ff28717a859fc9d61fe222e commit 5129b3f90b5069236ff28717a859fc9d61fe222e Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-01-27 18:12:33 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-10-03 05:14: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. (cherry picked from commit 6687410af7dba54e19b773684a969e9c590999e4) --- 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 0ff4d53f924b..85b76a44150c 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 @@ -219,14 +221,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 @@ -291,7 +290,7 @@ local function process_config(file) end end - io.close(fh) + assert(io.close(fh)) return cfg end @@ -320,7 +319,7 @@ local function grab_capenabled(file, open_fail_ok) end end - io.close(fh) + assert(io.close(fh)) return capentries end @@ -398,8 +397,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) @@ -407,13 +406,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 @@ -534,7 +533,7 @@ local function process_sysfile(file) process_syscall_def(prevline) end - io.close(fh) + assert(io.close(fh)) return capentries end @@ -1132,7 +1131,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] @@ -1140,13 +1139,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 @@ -1174,17 +1167,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 @@ -1382,12 +1386,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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110030515.1935Fjpo008829>