cleanup

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

util/libreboot-utils/include/common.h

@@ -83,7 +83,7 @@ int fchmod(int fd, mode_t mode);
 #endif
 
 #ifndef REAL_POS_IO
-#define REAL_POS_IO 1
+#define REAL_POS_IO 0
 #endif
 
 #ifndef LOOP_EAGAIN

util/libreboot-utils/lib/file.c

@@ -203,6 +203,7 @@ rw_file_exact(int fd, unsigned char *mem, size_t nrw,
 	size_t retries_on_zero;
 
 	int saved_errno = errno;
+	errno = 0;
 
 	rval = 0;
 
@@ -294,6 +295,11 @@ err_rw_file_exact:
  *              a change was detected, assuming
  *              nothing else is touching it now
  * off_reset 0: never reset if changed
+ *
+ * REAL_POS_IO is enabled by default in common.h
+ * and the fallback version was written for fun.
+ * You should just use the real one (REAL_POS_IO 1),
+ * since it is generally more reliable.
  */
 
 ssize_t
@@ -313,6 +319,7 @@ prw(int fd, void *mem, size_t nrw,
 	off_t off_last;
 #endif
 	int saved_errno = errno;
+	errno = 0;
 
 	if (io_args(fd, mem, nrw, off, rw_type)
 	    == -1)
@@ -542,7 +549,7 @@ lseek_on_eintr(int fd, off_t off, int whence,
 }
 
 /* two functions that reduce sloccount by
- * two hundred lines... no, now three. */
+ * two hundred lines */
 int
 if_err(int condition, int errval)
 {
@@ -554,13 +561,6 @@ if_err(int condition, int 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)
 {
@@ -616,10 +616,8 @@ close_warn(int *fd, char *s)
 	return 0;
 }
 
-/* TODO: remove this. giant liability.
-   make close calls always err instead,
-   when they fail. otherwise we hide bugs!
- */
+/* TODO: remove this, and just check
+ * err on every close. */
 void
 close_no_err(int *fd)
 {
@@ -636,7 +634,6 @@ close_no_err(int *fd)
 
 /* TODO: make fd a pointer insttead
 	and automatically reset -1 here */
-/* BUT DO NOT reset -1 on error */
 int
 close_on_eintr(int fd)
 {
@@ -683,11 +680,10 @@ fs_rename_at(int olddirfd, const char *old,
 	return renameat(olddirfd, old, newdirfd, new);
 }
 
-/* secure open, based on
- * relative path to root
+/* secure open, based on relative path to root
  *
- * always a fixed fd for /
- * see: rootfs()
+ * always a fixed fd for / see: rootfs()
+ * and fs_resolve_at()
  */
 int
 fs_open(const char *path, int flags)
@@ -702,12 +698,8 @@ fs_open(const char *path, int flags)
 	return fs_resolve_at(fs->rootfd, path + 1, flags);
 }
 
-/* singleton function
- * that returns a fixed
- * descriptor of /
- *
- * used throughout, for
- * repeated integrity checks
+/* singleton function that returns a fixed descriptor of /
+ * used throughout, for repeated integrity checks
  */
 struct filesystem *
 rootfs(void)
@@ -729,8 +721,7 @@ rootfs(void)
 	return &global_fs;
 }
 
-/* filesystem sandboxing.
- * (in userspace)
+/* filesystem sandboxing in userspace
  */
 int
 fs_resolve_at(int dirfd, const char *path, int flags)
@@ -769,10 +760,9 @@ fs_resolve_at(int dirfd, const char *path, int flags)
 		if (nextfd < 0)
 			goto err;
 
-		/* close previous fd IF it is not the original input */
-		if (curfd != dirfd) {
+		/* close previous fd if not the original input */
+		if (curfd != dirfd)
 			(void) close_on_eintr(curfd);
-		}
 
 		curfd = nextfd;
 		nextfd = -1;
@@ -921,9 +911,6 @@ fs_dirname_basename(const char *path,
 
 /* portable wrapper for use of openat2 on linux,
  * with fallback for others e.g. openbsd
- *
- * BONUS: arg checks
- * TODO: consider EINTR/EAGAIN retry loop
  */
 int
 openat2p(int dirfd, const char *path,
@@ -974,8 +961,7 @@ retry:
 }
 
 int
-mkdirat_on_eintr( /* <-- say that 10 times to please the demon */
-    int dirfd, 
+mkdirat_on_eintr(int dirfd, 
     const char *path, mode_t mode)
 {
 	int saved_errno = errno;

util/libreboot-utils/lib/mkhtemp.c

@@ -51,22 +51,6 @@ new_tmpdir(int *fd, char **path, char *tmpdir,
 	    tmpdir, template);
 }
 
-/* note: tmpdir is an override of TMPDIR or /tmp or /var/tmp */
-/* WARNING:
- * on error, *path (at **path) may be
-   NULL, or if the error pertains to
-   an actual TMPDIR, set. if set, it
-   will be using *static* memory and
-   must not be freed. on success,
-   a pointer to heap memory is set
-   instead.
- * see:
- * env_tmpdir()
- * this is for error reports if e.g.
- * TMPDIR isn't found (but is set)
- * if TMPDIR isn't set, it will
- * default to /tmp or /var/tmp
- */
 int
 new_tmp_common(int *fd, char **path, int type,
     char *tmpdir, const char *template)
@@ -443,20 +427,8 @@ world_writeable_and_sticky(
 		goto sticky_hell;
 	}
 
-	/* must be fully executable
-	 * by everyone, or openat2
-	 * becomes unreliable**
-	 *
-	 * TODO: loosen these, as a toggle.
-	 * execution rights isn't
-	 * really a requirement for
-	 * TMPDIR, except maybe search,
-	 * but this function will be
-	 * generalised at some point
-	 * for use in other tools
-	 * besides just mkhtemp.
-	 */
-	/*
+	/* all of these checks are probably
+	 * redundant (execution rights)
 	if (!(st.st_mode & S_IXUSR) ||
 	    !(st.st_mode & S_IXGRP) ||
 	    !(st.st_mode & S_IXOTH)) {
@@ -473,7 +445,7 @@ world_writeable_and_sticky(
 	if (bypass_all_sticky_checks)
 		goto sticky_heaven; /* normal == no security */
 
-	/* unhinged leah mode:
+	/* extremely not-libc mode:
 	 */
 
 	if (st.st_mode & S_IWOTH) { /* world writeable */
@@ -488,9 +460,7 @@ world_writeable_and_sticky(
 		goto sticky_hell; /* not sticky */
 	}
 
-	/* if anyone even looks at you funny, drop
-	 * everything on the floor and refuse to function
-	 */
+	/* for good measure */
 	if (faccessat(dirfd, 	".", X_OK, AT_EACCESS) < 0)
 		goto sticky_hell;
 
@@ -503,7 +473,6 @@ world_writeable_and_sticky(
 	goto sticky_hell; /* heaven visa denied */
 
 sticky_heaven:
-/* i like the one in hamburg better */
 
 	close_no_err(&dirfd);
 	errno = saved_errno;
@@ -515,10 +484,7 @@ sticky_hell:
 	if (errno == saved_errno)
 		errno = EPERM;
 
-	saved_errno = errno;
-
 	close_no_err(&dirfd);
-
 	errno = saved_errno;
 
 	return 0;
@@ -909,11 +875,9 @@ retry_rand:
 
 /* WARNING: **ONCE** per file.
  *
- *	!!! 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?
+ * some of these checks will trip up
+ * if you do them twice; all of them
+ * only need to be done once anyway.
  */
 int secure_file(int *fd,
     struct stat *st,
@@ -945,7 +909,7 @@ int secure_file(int *fd,
 	if (check_seek) {        /***********/
 		if (lseek(*fd, 0, SEEK_CUR) == (off_t)-1)
 			goto err_demons;
-	} /* don't release the demon */
+	} /* don't release the demon! */
 
 	if (if_err(st->st_nlink != 1, ELOOP) ||
 	    if_err(st->st_uid != geteuid() && geteuid() != 0, EPERM) ||

util/libreboot-utils/lib/num.c

@@ -1,12 +1,8 @@
 /* SPDX-License-Identifier: MIT
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
  *
- * Numerical functions.
- * NOTE: randomness was moved to rand.c
- */
-
-/*
-TODO: properly handle errno in this file
+ * Non-randomisation-related numerical functions.
+ * For rand functions, see: rand.c
  */
 
 #ifdef __OpenBSD__
@@ -27,44 +23,14 @@ TODO: properly handle errno in this file
 
 #include "../include/common.h"
 
-/* TODO:
- * make this and errno handling more
- * flexible
-
-  in particular:
-  hextonum could be modified to
-  write into a buffer instead,
-  with the converted numbers,
-  of an arbitrary length
- */
 unsigned short
 hextonum(char ch_s)
 {
 	int saved_errno = errno;
 
-	/* rlong() can return error,
-	   but preserves errno if no
-	   error. we need to detect
-	   this because it handles
-	   /dev/urandom sometimes
-
-	   therefore, if it's zero
-	   at start, we know if there
-	   was an err at the end, by
-	   return value zero, if errno
-	   was set; this is technically
-	   valid, since zero is also
-	   a valid random number!
-
-	it's an edge case that i had
-	to fix. i'll rewrite the code
-	better later. for now, it
-	should be ok.
-	 */
-	errno = 0;
-
 	unsigned char ch;
 	size_t rval;
+	errno = 0;
 
 	ch = (unsigned char)ch_s;