libreboot-utils: general code cleanup

Signed-off-by: Leah Rowe <leah@libreboot.org>

7 files changed, 211 insertions, 508 deletions

diff --git a/util/libreboot-utils/include/common.h b/util/libreboot-utils/include/common.h
index bdd625f8..b26448db 100644
--- a/util/libreboot-utils/include/common.h
+++ b/util/libreboot-utils/include/common.h
@@ -531,6 +531,8 @@ int fs_rename_at(int olddirfd, const char *old,
              int newdirfd, const char *new);
 int fs_open(const char *path, int flags);
 void close_no_err(int *fd);
+void free_if_null(char **buf);
+int close_warn(int *fd, char *s);
 struct filesystem *rootfs(void);
 int fs_resolve_at(int dirfd, const char *path, int flags);
 int fs_next_component(const char **p,
@@ -543,6 +545,8 @@ int openat2p(int dirfd, const char *path,
     int flags, mode_t mode);
 int mkdirat_on_eintr(int dirfd, 
     const char *pathname, mode_t mode);
+int if_err(int condition, int errval);
+int if_err_sys(int condition);
 
 /* asserts */
 
diff --git a/util/libreboot-utils/lib/file.c b/util/libreboot-utils/lib/file.c
index 00cab2e9..c9ec8f61 100644
--- a/util/libreboot-utils/lib/file.c
+++ b/util/libreboot-utils/lib/file.c
@@ -1,11 +1,12 @@
 /* SPDX-License-Identifier: MIT
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
  *
- * Pathless i/o, and some stuff you probably never saw.
+ * Pathless i/o, and some stuff you
+ * probably never saw in userspace.
+ *
  * Be nice to the demon.
  */
 
-
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -134,32 +135,14 @@ fsync_dir(const char *path)
 	maxlen = 4096;
 #endif
 
-	if (path == NULL) {
-		errno = EFAULT;
-		goto err_fsync_dir;
-	}
-
-	if (slen(path, maxlen, &pathlen) < 0)
+	if (if_err(path == NULL, EFAULT) ||
+	    if_err_sys(slen(path, maxlen, &pathlen) < 0) ||
+	    if_err(pathlen >= maxlen || pathlen < 0, EMSGSIZE) ||
+	    if_err(pathlen == 0, EINVAL)
+	    ||
+	    if_err_sys((dirbuf = malloc(pathlen + 1)) == NULL))
 		goto err_fsync_dir;
 
-	if (pathlen >= maxlen || pathlen < 0) {
-		errno = EMSGSIZE;
-		goto err_fsync_dir;
-	}
-
-	if (pathlen == 0)
-	{
-		errno = EINVAL;
-		goto err_fsync_dir;
-	}
-
-	dirbuf = malloc(pathlen + 1);
-	if (dirbuf == NULL) {
-
-		errno = ENOMEM;
-		goto err_fsync_dir;
-	}
-
 	memcpy(dirbuf, path, pathlen + 1);
 	slash = strrchr(dirbuf, '/');
 
@@ -183,20 +166,12 @@ fsync_dir(const char *path)
 	    | O_NOFOLLOW
 #endif
 );
-	if (dirfd < 0)
-		goto err_fsync_dir;
-
-	if (fstat(dirfd, &st) < 0)
-		goto err_fsync_dir;
-
-	if (!S_ISDIR(st.st_mode)) {
 
-		errno = ENOTDIR;
-		goto err_fsync_dir;
-	}
-
-	/* sync file on disk */
-	if (fsync_on_eintr(dirfd) == -1)
+	if (if_err_sys(dirfd < 0) ||
+	    if_err_sys(fstat(dirfd, &st) < 0) ||
+	    if_err(!S_ISDIR(st.st_mode), ENOTDIR)
+	    ||
+	    if_err_sys(fsync_on_eintr(dirfd) == -1))
 		goto err_fsync_dir;
 
 	if (close_on_eintr(dirfd) == -1) {
@@ -205,13 +180,7 @@ fsync_dir(const char *path)
 		goto err_fsync_dir;
 	}
 
-	if (dirbuf != NULL) {
-
-		free(dirbuf);
-		dirbuf = NULL;
-	}
-
-	dirbuf = NULL;
+	free_if_null(&dirbuf);
 
 	errno = saved_errno;
 	return 0;
@@ -221,19 +190,8 @@ err_fsync_dir:
 	if (errno == saved_errno)
 		errno = EIO;
 
-	if (dirbuf != NULL) {
-
-		free(dirbuf);
-		dirbuf = NULL;
-	}
-
-	if (dirfd >= 0) {
-
-		close_errno = errno;
-		(void) close_on_eintr(dirfd);
-		errno = close_errno;
-		dirfd = -1;
-	}
+	free_if_null(&dirbuf);
+	close_no_err(&dirfd);
 
 	return -1;
 }
@@ -545,57 +503,19 @@ io_args(int fd, void *mem, size_t nrw,
 {
 	int saved_errno = errno;
 
-	/* obviously */
-	if (mem == NULL) {
-
-		errno = EFAULT;
-		goto err_io_args;
-	}
-
-	/* uninitialised fd */
-	if (fd < 0) {
-
-		errno = EBADF;
+	if (if_err(mem == NULL, EFAULT) ||
+	    if_err(fd < 0, EBADF) ||
+	    if_err(off < 0, ERANGE) ||
+	    if_err(!nrw, EPERM) || /* TODO: toggle zero-byte check */
+	    if_err(nrw > (size_t)SSIZE_MAX, ERANGE) ||
+	    if_err(((size_t)off + nrw) < (size_t)off, ERANGE) ||
+	    if_err(rw_type > IO_PWRITE, EINVAL))
 		goto err_io_args;
-	}
-
-	/* negative offset */
-	if (off < 0) {
-
-		errno = ERANGE;
-		goto err_io_args;
-	}
-
-	/* prevent zero-byte rw */
-	if (!nrw)
-		goto err_io_args;
-
-	/* prevent overflow */
-	if (nrw > (size_t)SSIZE_MAX) {
-
-		errno = ERANGE;
-		goto err_io_args;
-	}
-
-	/* prevent overflow */
-	if (((size_t)off + nrw) < (size_t)off) {
-
-		errno = ERANGE;
-		goto err_io_args;
-	}
-
-	if (rw_type > IO_PWRITE) {
-
-		errno = EINVAL;
-		goto err_io_args;
-	}
 
 	errno = saved_errno;
-
 	return 0;
 
 err_io_args:
-
 	if (errno == saved_errno)
 		errno = EINVAL;
 
@@ -607,31 +527,16 @@ check_file(int fd, struct stat *st)
 {
 	int saved_errno = errno;
 
-	if (fd < 0) {
-		errno = EBADF;
-		goto err_is_file;
-	}
-
-	if (st == NULL) {
-		errno = EFAULT;
-		goto err_is_file;
-	}
-
-	if (fstat(fd, st) == -1)	
-		goto err_is_file;
-
-	if (!S_ISREG(st->st_mode)) {
-
-		errno = EBADF;
+	if (if_err(fd < 0, EBADF) ||
+	    if_err(st == NULL, EFAULT) ||
+	    if_err(fstat(fd, st) == -1, 0) ||
+	    if_err(!S_ISREG(st->st_mode), EBADF))
 		goto err_is_file;
-	}
 
 	errno = saved_errno;
-
 	return 0;
 
 err_is_file:
-
 	if (errno == saved_errno)
 		errno = EINVAL;
 
@@ -647,54 +552,16 @@ rw_over_nrw(ssize_t r, size_t nrw)
 {
 	int saved_errno = errno;
 
-	/* not a libc bug, but we
-	 * don't like the number zero
-	 */
-	if (!nrw)
-		goto err_rw_over_nrw;
-
-	if (r == -1)
-		return r;
-
-	if ((size_t)
-	    r > SSIZE_MAX) {
-
-		/* Theoretical buggy libc
-		 * check. Extremely academic.
-		 *
-		 * Specifications never
-		 * allow this return value
-		 * to exceed SSIZE_T, but
-		 * spec != implementation
-		 *
-		 * Check this after using
-		 * [p]read() or [p]write()
-		 *
-		 * NOTE: here, we assume
-		 * ssize_t integers are the
-		 * same size as SSIZE_T
-		 */
-
-		errno = ERANGE;
+	if (if_err(!nrw, 0) ||
+	    if_err(r == -1, 0) ||
+	    if_err((size_t)r > SSIZE_MAX, ERANGE) ||
+	    if_err((size_t)r > nrw, ERANGE))
 		goto err_rw_over_nrw;
-	}
-
-	/* Theoretical buggy libc:
-	 * Should never return a number of
-	 * bytes above the requested length.
-	 */
-	if ((size_t)r > nrw) {
-
-		errno = ERANGE;
-		goto err_rw_over_nrw;
-	}
 
 	errno = saved_errno;
-
 	return r;
 
 err_rw_over_nrw:
-
 	if (errno == saved_errno)
 		errno = EIO;
 
@@ -723,23 +590,91 @@ lseek_on_eintr(int fd, off_t off, int whence,
 }
 #endif
 
+/* two functions that reduce sloccount by
+ * two hundred lines... no, now three. */
+int
+if_err(int condition, int errval)
+{
+	if (!condition)
+		return 0;
+
+	if (errval)
+		errno = errval;
+
+	return 1;
+}
+/* technically pointless, but stylistically
+ * pleasing alongside if_err chains.
+ * use this one for syscalls that are
+ * expected to set errno
+ * also use it for non-system calls
+ * that act like them, e.g. prw() or
+ * rw_write_exact() */
+int
+if_err_sys(int condition)
+{
+	if (!condition)
+		return 0;
+	return 1;
+}
+/* errno can never be -1, so you can
+ * use this to conditionally set an integer
+ * for comparison. see example in lseek_on_eintr
+ */
 int
 try_err(int loop_err, int errval)
 {
 	if (loop_err)
 		return errval;
-
 	return -1;
 }
 
 void
+free_if_null(char **buf)
+{
+	if (buf == NULL || *buf == NULL)
+		return;
+
+	free(*buf);
+	*buf = NULL;
+}
+
+/* also returns error code */
+int
+close_warn(int *fd, char *s)
+{
+	int saved_errno = errno;
+
+	if (fd == NULL) {
+		if (s != NULL)
+			fprintf(stderr, "FAIL: %s: bad fd ptr\n", s);
+		return -1;
+	}
+
+	if (*fd < 0 && s != NULL) {
+		fprintf(stderr, "WARN: %s: already closed\n", s);
+	} else if (close(*fd) < 0) {
+		if (s != NULL)
+			fprintf(stderr, "FAIL: %s: close\n", s);
+		return -1;
+	}
+
+	*fd = -1;
+	errno = saved_errno;
+
+	return 0;
+}
+
+/* TODO: remove this. giant liability.
+   make close calls always err instead,
+   when they fail. otherwise we hide bugs!
+ */
+void
 close_no_err(int *fd)
 {
 	int saved_errno = errno;
 
-	if (fd == NULL)
-		return;
-	if (*fd < 0)
+	if (fd == NULL || *fd < 0)
 		return;
 
 	(void) close_on_eintr(*fd);
@@ -748,6 +683,9 @@ close_no_err(int *fd)
 	errno = saved_errno;
 }
 
+/* TODO: make fd a pointer insttead
+	and automatically reset -1 here */
+/* BUT DO NOT reset -1 on error */
 int
 close_on_eintr(int fd)
 {
@@ -787,17 +725,9 @@ int
 fs_rename_at(int olddirfd, const char *old,
     int newdirfd, const char *new)
 {
-	if (new == NULL || old == NULL) {
-
-		errno = EFAULT;
+	if (if_err(new == NULL || old == NULL, EFAULT) ||
+	    if_err(olddirfd < 0 || newdirfd < 0, EBADF))
 		return -1;
-	}
-
-	if (olddirfd < 0 || newdirfd < 0) {
-
-		errno = EBADF;
-		return -1;
-	}
 
 	return renameat(olddirfd, old, newdirfd, new);
 }
@@ -812,25 +742,13 @@ int
 fs_open(const char *path, int flags)
 {
 	struct filesystem *fs;
-	const char *rel;
-
-	if (path == NULL) {
-		errno = EFAULT;
-		return -1;
-	}
-
-	if (path[0] != '/') {
-		errno = EINVAL;
-		return -1;
-	}
 
-	fs = rootfs();
-	if (fs == NULL)
+	if (if_err(path == NULL, EFAULT) ||
+	    if_err(path[0] != '/', EINVAL) ||
+	    if_err_sys((fs = rootfs()) == NULL))
 		return -1;
 
-	rel = path + 1;
-
-	return fs_resolve_at(fs->rootfd, rel, flags);
+	return fs_resolve_at(fs->rootfd, path + 1, flags);
 }
 
 /* singleton function
@@ -980,12 +898,8 @@ fs_open_component(int dirfd, const char *name,
 	 */
 	if (!is_last) {
 
-		if (fd < 0) {
-			errno = EBADF;
-			return -1;
-		}
-
-		if (fstat(fd, &st) < 0)
+		if (if_err(fd < 0, EBADF) ||
+		    if_err_sys(fstat(fd, &st) < 0))
 			return -1;
 
 		if (!S_ISDIR(st.st_mode)) {
@@ -1015,14 +929,9 @@ fs_dirname_basename(const char *path,
 	size_t maxlen = 4096;
 #endif
 
-	if (path == NULL || dir == NULL || base == NULL)
-		return -1;
-
-	if (slen(path, maxlen, &len) < 0)
-		return -1;
-
-	buf = malloc(len + 1);
-	if (buf == NULL)
+	if (path == NULL || dir == NULL || base == NULL ||
+	    if_err_sys(slen(path, maxlen, &len) < 0) ||
+	    if_err_sys((buf = malloc(len + 1)) == NULL))
 		return -1;
 
 	memcpy(buf, path, len + 1);
@@ -1049,7 +958,7 @@ fs_dirname_basename(const char *path,
 		*base = buf;
 	} else {
 		errno = EINVAL;
-		free(buf);
+		free_if_null(&buf);
 		return -1;
 	}
 
@@ -1079,15 +988,9 @@ openat2p(int dirfd, const char *path,
 	int rval;
 #endif
 
-	if (dirfd < 0) {
-		errno = EBADF;
-		return -1;
-	}
-
-	if (path == NULL) {
-		errno = EFAULT;
+	if (if_err(dirfd < 0, EBADF) ||
+	    if_err(path == NULL, EFAULT))
 		return -1;
-	}
 
 retry:
 	errno = 0;
@@ -1124,15 +1027,9 @@ mkdirat_on_eintr( /* <-- say that 10 times to please the demon */
 	int saved_errno = errno;
 	int rval;
 
-	if (dirfd < 0) {
-		errno = EBADF;
-		return -1;
-	}
-
-	if (path == NULL) {
-		errno = EFAULT;
+	if (if_err(dirfd < 0, EBADF) ||
+	    if_err(path == NULL, EFAULT))
 		return -1;
-	}
 
 retry:
 	errno = 0;
diff --git a/util/libreboot-utils/lib/io.c b/util/libreboot-utils/lib/io.c
index 94bde87e..cc38e5c7 100644
--- a/util/libreboot-utils/lib/io.c
+++ b/util/libreboot-utils/lib/io.c
@@ -239,21 +239,8 @@ write_to_gbe_bin(void)
 
 	saved_errno = errno;
 
-	if (close_on_eintr(f->tmp_fd) == -1) {
-		f->tmp_fd = -1;
-
-		fprintf(stderr, "FAIL: %s: close\n", f->tname);
-		f->io_err_gbe_bin = 1;
-	}
-	f->tmp_fd = -1;
-
-	if (close_on_eintr(f->gbe_fd) == -1) {
-		f->gbe_fd = -1;
-
-		fprintf(stderr, "FAIL: %s: close\n", f->fname);
-		f->io_err_gbe_bin = 1;
-	}
-	f->gbe_fd = -1;
+	f->io_err_gbe_bin |= -close_warn(&f->tmp_fd, f->tname);
+	f->io_err_gbe_bin |= -close_warn(&f->gbe_fd, f->fname);
 
 	errno = saved_errno;
 
@@ -279,13 +266,7 @@ write_to_gbe_bin(void)
 
 			/* removed by rename
 			 */
-
-			if (f->tname != NULL) {
-				free(f->tname);
-				f->tname = NULL;
-			}
-
-			f->tname = NULL;
+			free_if_null(&f->tname);
 		}
 	}
 
@@ -513,12 +494,7 @@ gbe_mv(void)
 		goto ret_gbe_mv;
 	}
 
-	if (dest_tmp != NULL) {
-		free(dest_tmp);
-		dest_tmp = NULL;
-	}
-
-	dest_tmp = NULL;
+	free_if_null(&dest_tmp);
 
 ret_gbe_mv:
 
diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c
index 8f54b045..b30f6587 100644
--- a/util/libreboot-utils/lib/mkhtemp.c
+++ b/util/libreboot-utils/lib/mkhtemp.c
@@ -168,10 +168,7 @@ err:
 	else
 		saved_errno = errno = EIO;
 
-	if (dest != NULL) {
-		free(dest);
-		dest = NULL;
-	}
+	free_if_null(&dest);
 
 	close_no_err(&dirfd);
 	close_no_err(fd);
@@ -543,76 +540,32 @@ mkhtemp(int *fd,
 	int r;
 	char *end;
 
-	if (fd == NULL ||
-	    template == NULL ||
-	    fname == NULL ||
-	    st_dir_initial == NULL) {
-
-		errno = EFAULT;
-		return -1;
-	}
-
-	/* we do not mess with an
-	   open descriptor.
-	 */
-	if (*fd >= 0) {
-		errno = EEXIST; /* leave their file alone */
-		return -1;
-	}
-
-	if (dirfd < 0) {
-		errno = EBADF;
-		return -1;
-	}
-
-	if (slen(template, max_len, &len) < 0)
-		return -1;
-
-	if (len >= max_len) {
-		errno = EMSGSIZE;
+	if (if_err(fd == NULL || template == NULL || fname == NULL ||
+	          st_dir_initial == NULL, EFAULT) ||
+	    if_err(*fd >= 0, EEXIST) ||
+	    if_err(dirfd < 0, EBADF)
+	      ||
+	    if_err_sys(slen(template, max_len, &len) < 0) ||
+	    if_err(len >= max_len, EMSGSIZE)
+	      ||
+	    if_err_sys(slen(fname, max_len, &fname_len)) ||
+	    if_err(fname == 0, EINVAL) ||
+	    if_err(strrchr(fname, '/') != NULL, EINVAL))
 		return -1;
-	}
-
-	if (slen(fname, max_len, &fname_len) < 0)
-		return -1;
-
-	if (fname_len == 0) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	if (strrchr(fname, '/') != NULL) {
-		errno = EINVAL;
-		return -1;
-	}
 
-	/* count trailing X */
-	end = template + len;
-	while (end > template && *--end == 'X')
-		xc++;
+	for (end = template + len; /* count X */
+	    end > template && *--end == 'X'; xc++);
 
-	if (xc < 6 || xc > len) {
-		errno = EINVAL;
+	if (if_err(xc < 6 || xc > len, EINVAL) ||
+	    if_err(fname_len > len, EOVERFLOW))
 		return -1;
-	}
 
-	if (fname_len > len) {
-		errno = EOVERFLOW;
+	if (if_err(memcmp(fname, template + len - fname_len,
+	      fname_len) != 0, EINVAL))
 		return -1;
-	}
 
-	if (memcmp(fname,
-	    template + len - fname_len,
-	    fname_len) != 0) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	fname_copy = malloc(fname_len + 1);
-	if (fname_copy == NULL) {
-		errno = ENOMEM;
+	if((fname_copy = malloc(fname_len + 1)) == NULL)
 		goto err;
-	}
 
 	/* fname_copy = suffix region only; p points to trailing XXXXXX */
 	memcpy(fname_copy,
@@ -622,15 +575,9 @@ mkhtemp(int *fd,
 
 	for (retries = 0; retries < MKHTEMP_RETRY_MAX; retries++) {
 
-		r = mkhtemp_try_create(
-		    dirfd,
-		    st_dir_initial,
-		    fname_copy,
-		    p,
-		    xc,
-		    fd,
-		    st,
-		    type);
+		r = mkhtemp_try_create(dirfd,
+		    st_dir_initial, fname_copy,
+		    p, xc, fd, st, type);
 
 		if (r == 0) {
 			if (retries >= MKHTEMP_SPIN_THRESHOLD) {
@@ -658,9 +605,7 @@ err:
 	close_no_err(fd);
 
 success:
-
-	if (fname_copy != NULL)
-		free(fname_copy);
+	free_if_null(&fname_copy);
 
 	return (*fd >= 0) ? *fd : -1;
 }
@@ -682,29 +627,21 @@ mkhtemp_try_create(int dirfd,
 	int file_created = 0;
 	int dir_created = 0;
 
-	if (fd == NULL || st == NULL || p == NULL || fname_copy == NULL ||
-	    st_dir_initial == NULL) {
-		errno = EFAULT;
-		goto err;
-	} else if (*fd >= 0) { /* do not mess with someone else's file */
-		errno = EEXIST;
-		goto err;
-	}
-
-	if (mkhtemp_fill_random(p, xc) < 0)
+	if (if_err(fd == NULL || st == NULL || p ==NULL || fname_copy ==NULL ||
+	      st_dir_initial == NULL, EFAULT) ||
+	    if_err(*fd >= 0, EEXIST))
 		goto err;
 
-	if (fd_verify_dir_identity(dirfd, st_dir_initial) < 0)
+	if (if_err_sys(mkhtemp_fill_random(p, xc) < 0) ||
+	    if_err_sys(fd_verify_dir_identity(dirfd, st_dir_initial) < 0))
 		goto err;
 
 	if (type == MKHTEMP_FILE) {
 		*fd = openat2p(dirfd, fname_copy,
 		    O_RDWR | O_CREAT | O_EXCL |
-		    O_NOFOLLOW | O_CLOEXEC | O_NOCTTY,
-		    0600);
+		    O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, 0600);
 
-		/* O_CREAT and O_EXCL guarantees
-		 * creation upon success
+		/* O_CREAT and O_EXCL guarantees creation upon success
 		 */
 		if (*fd >= 0)
 			file_created = 1;
@@ -724,23 +661,15 @@ mkhtemp_try_create(int dirfd,
 		if (fd_verify_dir_identity(dirfd, st_dir_initial) < 0)
 			goto err;
 
-		*fd = openat2p(dirfd, fname_copy,
-		    O_RDONLY | O_DIRECTORY | O_CLOEXEC, 0);
-		if (*fd < 0)
+		if ((*fd = openat2p(dirfd, fname_copy,
+		    O_RDONLY | O_DIRECTORY | O_CLOEXEC, 0)) < 0)
 			goto err;
 
-		if (fstat(*fd, &st_open) < 0)
+		if (if_err_sys(fstat(*fd, &st_open) < 0) ||
+		    if_err(!S_ISDIR(st_open.st_mode), ENOTDIR))
 			goto err;
 
-		if (!S_ISDIR(st_open.st_mode)) {
-			errno = ENOTDIR;
-			goto err;
-		}
-
-		/* NOTE: could check nlink count here,
-		 *       but it's not very reliable here. skipped.
-		 */
-
+		/* NOTE: pointless to check nlink here (only just opened) */
 		if (fd_verify_dir_identity(dirfd, st_dir_initial) < 0)
 			goto err;
 
@@ -776,19 +705,10 @@ mkhtemp_try_create(int dirfd,
 		if (fd_verify_identity(*fd, &st_open, st_dir_initial) < 0)
 			goto err;
 
-		if (!S_ISDIR(st_open.st_mode)) {
-			errno = ENOTDIR;
-			goto err;
-		}
-
-		if (is_owner(&st_open) < 0)
-			goto err;
-
-		/* group/world writeable */
-		if (st_open.st_mode & (S_IWGRP | S_IWOTH)) {
-			errno = EPERM;
+		if (if_err(!S_ISDIR(st_open.st_mode), ENOTDIR) ||
+		    if_err_sys(is_owner(&st_open) < 0) ||
+		    if_err(st_open.st_mode & (S_IWGRP | S_IWOTH), EPERM))
 			goto err;
-		}
 	}
 
 	errno = saved_errno;
@@ -800,7 +720,6 @@ err:
 
 	if (file_created)
 		(void) unlinkat(dirfd, fname_copy, 0);
-
 	if (dir_created)
 		(void) unlinkat(dirfd, fname_copy, AT_REMOVEDIR);
 
@@ -879,6 +798,9 @@ err_mkhtemp_fill_random:
  *	!!! DO NOT RUN TWICE PER FILE. BEWARE OF THE DEMON !!!
  *	               watch out for spikes!
  */
+/* TODO: bad_flags can be negative, and is
+ *       ignored if it is. should we err instead?
+ */
 int secure_file(int *fd,
     struct stat *st,
     struct stat *expected,
@@ -890,93 +812,40 @@ int secure_file(int *fd,
 	int flags;
 	struct stat st_now;
 	int saved_errno = errno;
-	/* you have been warned */
-	if (fd == NULL) {
-		errno = EFAULT;
-		goto err_demons;
-	}
-	if (*fd < 0) {
-		errno = EBADF;
-		goto err_demons;
-	}
-
-	if (st == NULL) {
-		errno = EFAULT;
-		goto err_demons;
-	}
-
-	flags = fcntl(*fd, F_GETFL);
 
-	if (flags == -1)
+	if (if_err(fd == NULL || st == NULL, EFAULT) ||
+	    if_err(*fd < 0, EBADF) ||
+	    if_err_sys((flags = fcntl(*fd, F_GETFL)) == -1) ||
+	    if_err(bad_flags > 0 && (flags & bad_flags), EPERM))
 		goto err_demons;
 
-	if (bad_flags > 0) {
-
-		/* e.g. O_APPEND breaks pwrite/pread
-		 * by allowing offsets to be ignored */
-		if (flags & bad_flags) {
-			errno = EPERM;
-			goto err_demons;
-		}
-	}
-
 	if (expected != NULL) {
 		if (fd_verify_regular(*fd, expected, st) < 0)
 			goto err_demons;
-	} else {
-		if (fstat(*fd, &st_now) == -1)
-			goto err_demons;
-
-		if (!S_ISREG(st_now.st_mode)) {
-			errno = EBADF;
-			goto err_demons;
-		}
-
-		*st = st_now;
-	}
-
-	if (check_seek) {
-
-		/* check if it's seekable */
+	} else if (if_err_sys(fstat(*fd, &st_now) == -1) ||
+	    if_err(!S_ISREG(st_now.st_mode), EBADF)) {
+		goto err_demons; /***********/
+	} else                   /* ( >:3 ) */
+		*st = st_now;    /*  /| |\  */  /* don't let him out */
+				 /*   / \   */
+	if (check_seek) {        /***********/
 		if (lseek(*fd, 0, SEEK_CUR) == (off_t)-1)
 			goto err_demons;
-	}
-
-	/* don't release the demon
-	 */
-	if (st->st_nlink != 1) { /***********/
-		                 /* ( >:3 ) */
-		errno = ELOOP;   /*  /| |\  */  /* don't let him out */
-		goto err_demons; /*   / \   */
-	                         /***********/
-	}
-
-	if (st->st_uid != geteuid() && /* someone else's file */
-	    geteuid() != 0) { /* override for root */
+	} /* don't release the demon */
 
-		errno = EPERM;
-		goto err_demons;
-	}
-	if (is_owner(st) < 0)
+	if (if_err(st->st_nlink != 1, ELOOP) ||
+	    if_err(st->st_uid != geteuid() && geteuid() != 0, EPERM) ||
+	    if_err_sys(is_owner(st) < 0) ||
+	    if_err(st->st_mode & (S_IWGRP | S_IWOTH), EPERM))
 		goto err_demons;
 
-	/* world-writeable or group-ownership.
-	 * if these are set, then others could
-	 * modify the file (not secure)
-	 */
-	if (st->st_mode & (S_IWGRP | S_IWOTH)) {
-		errno = EPERM;
-		goto err_demons;
-	}
-
 	if (do_lock) {
 		if (lock_file(*fd, flags) == -1)
 			goto err_demons;
 
-		if (expected != NULL) {
+		if (expected != NULL)
 			if (fd_verify_identity(*fd, expected, &st_now) < 0)
 				goto err_demons;
-		}
 	}
 
 	if (fchmod(*fd, mode) == -1)
@@ -997,16 +866,12 @@ int
 fd_verify_regular(int fd,
     const struct stat *expected,
     struct stat *out)
-{
-	if (fd_verify_identity(fd, expected, out) < 0)
-		return -1;
-
-	if (!S_ISREG(out->st_mode)) {
-		errno = EBADF;
-		return -1;
-	}
-
-	return 0; /* regular file */
+{if (
+	if_err_sys(fd_verify_identity(fd, expected, out) < 0) ||
+	    if_err(!S_ISREG(out->st_mode), EBADF)
+	)	return -1;
+	else
+		return 0; /* regular file */
 }
 
 int
@@ -1017,20 +882,12 @@ fd_verify_identity(int fd,
 	struct stat st_now;
 	int saved_errno = errno;
 
-	if (fd < 0 || expected == NULL) {
-		errno = EFAULT;
-		return -1;
-	}
-
-	if (fstat(fd, &st_now) < 0)
+if(	if_err(fd < 0 || expected == NULL, EFAULT) ||
+	  if_err_sys(fstat(fd, &st_now)) ||
+	    if_err(st_now.st_dev != expected->st_dev ||
+	      st_now.st_ino != expected->st_ino, ESTALE))
 		return -1;
 
-	if (st_now.st_dev != expected->st_dev ||
-	    st_now.st_ino != expected->st_ino) {
-		errno = ESTALE;
-		return -1;
-	}
-
 	if (out != NULL)
 		*out = st_now;
 
@@ -1045,12 +902,8 @@ fd_verify_dir_identity(int fd,
 	struct stat st_now;
 	int saved_errno = errno;
 
-	if (fd < 0 || expected == NULL) {
-		errno = EFAULT;
-		return -1;
-	}
-
-	if (fstat(fd, &st_now) < 0)
+	if (if_err(fd < 0 || expected == NULL, EFAULT) ||
+	    if_err_sys(fstat(fd, &st_now) < 0))
 		return -1;
 
 	if (st_now.st_dev != expected->st_dev ||
@@ -1096,15 +949,9 @@ lock_file(int fd, int flags)
 	struct flock fl;
 	int saved_errno = errno;
 
-	if (fd < 0) {
-		errno = EBADF;
+	if (if_err(fd < 0, EBADF) ||
+	    if_err(flags < 0, EINVAL))
 		goto err_lock_file;
-	}
-
-	if (flags < 0) {
-		errno = EINVAL;
-		goto err_lock_file;
-	}
 
 	memset(&fl, 0, sizeof(fl));
 
diff --git a/util/libreboot-utils/lib/num.c b/util/libreboot-utils/lib/num.c
index 343350b0..91a1514f 100644
--- a/util/libreboot-utils/lib/num.c
+++ b/util/libreboot-utils/lib/num.c
@@ -257,12 +257,9 @@ retry_urandom_read:
 		nr = new_nr;
 		off = 0;
 
-		/* to mitigate file descriptor
-		 * injection, we do not re-use
-		 * the same descriptor each time
-		 */
-		(void) close_on_eintr(fd);
-		fd = -1;
+		/* don't re-use same fd, to mitaget
+		 * fd injection */
+		close_no_err(&fd);
 	}
 
 	fd = -1;
diff --git a/util/libreboot-utils/lib/state.c b/util/libreboot-utils/lib/state.c
index a3ad0f1e..b7701f0d 100644
--- a/util/libreboot-utils/lib/state.c
+++ b/util/libreboot-utils/lib/state.c
@@ -204,28 +204,13 @@ exit_cleanup(void)
 	if (x != NULL) {
 		f = &x->f;
 
-		if (f->gbe_fd > -1) {
-			if (close_on_eintr(f->gbe_fd) == -1) {
-				f->gbe_fd = -1;
-				close_err = 1;
-			}
-			f->gbe_fd = -1;
-		}
-
-		if (f->tmp_fd > -1) {
-			if (close_on_eintr(f->tmp_fd) == -1) {
-				f->tmp_fd = -1;
-				close_err = 1;
-			}
-			f->tmp_fd = -1;
-		}
+		close_no_err(&f->gbe_fd);
+		close_no_err(&f->tmp_fd);
+		close_no_err(&f->tmp_fd);
 
-		if (f->tname != NULL) {
+		if (f->tname != NULL)
 			if (unlink(f->tname) == -1)
 				close_err = 1;
-		}
-
-		f->tmp_fd = -1;
 	}
 
 	if (saved_errno)
diff --git a/util/libreboot-utils/nvmutil.c b/util/libreboot-utils/nvmutil.c
index fa6384b0..fb45a97c 100644
--- a/util/libreboot-utils/nvmutil.c
+++ b/util/libreboot-utils/nvmutil.c
@@ -123,10 +123,7 @@ main(int argc, char *argv[])
 	if (f->io_err_gbe_bin)
 		err(EIO, "%s: error writing final file");
 
-	if (f->tname != NULL) {
-		free(f->tname);
-		f->tname = NULL;
-	}
+	free_if_null(&f->tname);
 
 	return EXIT_SUCCESS;
 }