libreboot-utils: general code cleanup

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

1 files changed, 123 insertions, 226 deletions

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;