From owner-dev-commits-src-branches@freebsd.org Sun Oct 3 05:15:45 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 6F6F26B0B5F; Sun, 3 Oct 2021 05:15:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HMX9s2Wpgz4RL1; Sun, 3 Oct 2021 05:15:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 20A0D2062A; Sun, 3 Oct 2021 05:15:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1935Fjkd008830; Sun, 3 Oct 2021 05:15:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1935Fjpo008829; Sun, 3 Oct 2021 05:15:45 GMT (envelope-from git) Date: Sun, 3 Oct 2021 05:15:45 GMT Message-Id: <202110030515.1935Fjpo008829@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kyle Evans Subject: git: 5129b3f90b50 - stable/13 - makesyscalls: sprinkle some assert() on standard function calls MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 5129b3f90b5069236ff28717a859fc9d61fe222e Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Oct 2021 05:15:45 -0000 The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=5129b3f90b5069236ff28717a859fc9d61fe222e commit 5129b3f90b5069236ff28717a859fc9d61fe222e Author: Kyle Evans AuthorDate: 2021-01-27 18:12:33 +0000 Commit: Kyle Evans 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 ") + error("usage: " .. arg[0] .. " input-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