From owner-dev-commits-src-all@freebsd.org Sat Sep 25 01:59:03 2021 Return-Path: Delivered-To: dev-commits-src-all@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 88B10678B2D; Sat, 25 Sep 2021 01:59:03 +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 4HGXBb3CSdz4mk7; Sat, 25 Sep 2021 01:59:03 +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 3FACD21C72; Sat, 25 Sep 2021 01:59:03 +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 18P1x3Cf075854; Sat, 25 Sep 2021 01:59:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 18P1x3vq075853; Sat, 25 Sep 2021 01:59:03 GMT (envelope-from git) Date: Sat, 25 Sep 2021 01:59:03 GMT Message-Id: <202109250159.18P1x3vq075853@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kyle Evans Subject: git: 6687410af7db - main - 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/main X-Git-Reftype: branch X-Git-Commit: 6687410af7dba54e19b773684a969e9c590999e4 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Sep 2021 01:59:03 -0000 The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=6687410af7dba54e19b773684a969e9c590999e4 commit 6687410af7dba54e19b773684a969e9c590999e4 Author: Kyle Evans AuthorDate: 2021-01-27 18:12:33 +0000 Commit: Kyle Evans 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 ") + error("usage: " .. arg[0] .. " input-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