util/nvmutil: major cleanup

handle init in xstatus()

it's now a singleton design

also tidied up some other code

also removed todo.c. bloat.
will do all those anyway.

too much change. i just kept
touching the code until it
looked good

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

util/nvmutil/lib/checksum.c

@@ -29,9 +29,9 @@
 void
 read_checksums(void)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
 	unsigned long _p;
 	unsigned long _skip_part;
@@ -39,9 +39,6 @@ read_checksums(void)
 	unsigned char _num_invalid;
 	unsigned char _max_invalid;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	f->part_valid[0] = 0;
 	f->part_valid[1] = 0;
 

util/nvmutil/lib/command.c

@@ -24,13 +24,13 @@
 
 #include "../include/common.h"
 
-/*
- * Guard against regressions by maintainers (command table)
+/* Guard against regressions by maintainers (command table)
  */
+
 void
 sanitize_command_list(void)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
 
 	unsigned long c;
 	unsigned long num_commands;
@@ -41,20 +41,18 @@ sanitize_command_list(void)
 		sanitize_command_index(c);
 }
 
-/*
- * TODO: specific config checks per command
+/* TODO: specific config checks per command
  */
+
 void
 sanitize_command_index(unsigned long c)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[c];
 
 	int _flag;
 	unsigned long gbe_rw_size;
 
-	cmd = &x->cmd[c];
-
 	check_command_num(c);
 
 	if (cmd->argc < 3)
@@ -108,7 +106,7 @@ sanitize_command_index(unsigned long c)
 void
 set_cmd(int argc, char *argv[])
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
 	const char *cmd;
 
 	unsigned long c;
@@ -139,14 +137,10 @@ set_cmd(int argc, char *argv[])
 void
 set_cmd_args(int argc, char *argv[])
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
-	unsigned long i;
-
-	i = x->i;
-	cmd = &x->cmd[i];
-	f = &x->f;
+	struct xstate *x = xstatus(0, NULL);
+	unsigned long i = x->i;
+	struct commands *cmd = &x->cmd[i];
+	struct xfile *f = &x->f;
 
 	if (!valid_command(i) || argc < 3)
 		usage();
@@ -191,26 +185,6 @@ conv_argv_part_num(const char *part_str)
 
 	return (unsigned long)(ch - '0');
 }
-void
-run_cmd(void)
-{
-	struct xstate *x = xstatus();
-	unsigned long i;
-	void (*run)(void);
-
-	i = x->i;
-	run = x->cmd[i].run;
-
-	check_command_num(i);
-
-	if (run == NULL)
-		err(EINVAL, "Command %lu: null ptr", i);
-
-	run();
-
-	for (i = 0; i < items(x->cmd); i++)
-		x->cmd[i].run = cmd_helper_err;
-}
 
 void
 check_command_num(unsigned long c)
@@ -223,7 +197,7 @@ check_command_num(unsigned long c)
 unsigned char
 valid_command(unsigned long c)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
 	struct commands *cmd;
 
 	if (c >= items(x->cmd))
@@ -242,11 +216,10 @@ valid_command(unsigned long c)
 void
 cmd_helper_setmac(void)
 {
-	struct xstate *x = xstatus();
-	unsigned long partnum;
-	struct macaddr *mac;
+	struct xstate *x = xstatus(0, NULL);
+	struct macaddr *mac = &x->mac;
 
-	mac = &x->mac;
+	unsigned long partnum;
 
 	check_cmd(cmd_helper_setmac, "setmac");
 
@@ -260,13 +233,11 @@ cmd_helper_setmac(void)
 void
 parse_mac_string(void)
 {
-	struct xstate *x = xstatus();
-	struct macaddr *mac;
+	struct xstate *x = xstatus(0, NULL);
+	struct macaddr *mac = &x->mac;
 
 	unsigned long mac_byte;
 
-	mac = &x->mac;
-
 	if (xstrxlen(x->mac.str, 18) != 17)
 		err(EINVAL, "MAC address is the wrong length");
 
@@ -285,16 +256,14 @@ parse_mac_string(void)
 void
 set_mac_byte(unsigned long mac_byte_pos)
 {
-	struct xstate *x = xstatus();
-	struct macaddr *mac;
+	struct xstate *x = xstatus(0, NULL);
+	struct macaddr *mac = &x->mac;
 
 	char separator;
 
 	unsigned long mac_str_pos;
 	unsigned long mac_nib_pos;
 
-	mac = &x->mac;
-
 	mac_str_pos = mac_byte_pos * 3;
 
 	if (mac_str_pos < 15) {
@@ -311,14 +280,12 @@ void
 set_mac_nib(unsigned long mac_str_pos,
     unsigned long mac_byte_pos, unsigned long mac_nib_pos)
 {
-	struct xstate *x = xstatus();
-	struct macaddr *mac;
+	struct xstate *x = xstatus(0, NULL);
+	struct macaddr *mac = &x->mac;
 
 	char mac_ch;
 	unsigned short hex_num;
 
-	mac = &x->mac;
-
 	mac_ch = mac->str[mac_str_pos + mac_nib_pos];
 
 	if ((hex_num = hextonum(mac_ch)) > 15)
@@ -345,15 +312,12 @@ set_mac_nib(unsigned long mac_str_pos,
 void
 write_mac_part(unsigned long partnum)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
-	struct macaddr *mac;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
+	struct macaddr *mac = &x->mac;
 
 	unsigned long w;
 
-	f = &x->f;
-	mac = &x->mac;
-
 	check_bin(partnum, "part number");
 	if (!f->part_valid[partnum])
 		return;
@@ -369,13 +333,11 @@ write_mac_part(unsigned long partnum)
 void
 cmd_helper_dump(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	unsigned long p;
 
-	f = &x->f;
-
 	check_cmd(cmd_helper_dump, "dump");
 
 	f->part_valid[0] = good_checksum(0);
@@ -408,10 +370,13 @@ print_mac_from_nvm(unsigned long partnum)
 	unsigned short val16;
 
 	for (c = 0; c < 3; c++) {
+
 		val16 = nvm_word(c, partnum);
+
 		printf("%02x:%02x",
 		    (unsigned int)(val16 & 0xff),
 		    (unsigned int)(val16 >> 8));
+
 		if (c == 2)
 			printf("\n");
 		else
@@ -451,13 +416,11 @@ hexdump(unsigned long partnum)
 void
 cmd_helper_swap(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	check_cmd(cmd_helper_swap, "swap");
 
-	f = &x->f;
-
 	x_v_memcpy(
 	    f->buf + (unsigned long)GBE_WORK_SIZE,
 	    f->buf,
@@ -480,13 +443,11 @@ cmd_helper_swap(void)
 void
 cmd_helper_copy(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	check_cmd(cmd_helper_copy, "copy");
 
-	f = &x->f;
-
 	x_v_memcpy(
 	    f->buf + (unsigned long)((f->part ^ 1) * GBE_PART_SIZE),
 	    f->buf + (unsigned long)(f->part * GBE_PART_SIZE),
@@ -498,7 +459,8 @@ cmd_helper_copy(void)
 void
 cmd_helper_cat(void)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
+
 	check_cmd(cmd_helper_cat, "cat");
 
 	x->cat = 0;
@@ -508,7 +470,8 @@ cmd_helper_cat(void)
 void
 cmd_helper_cat16(void)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
+
 	check_cmd(cmd_helper_cat16, "cat16");
 
 	x->cat = 1;
@@ -518,7 +481,8 @@ cmd_helper_cat16(void)
 void
 cmd_helper_cat128(void)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
+
 	check_cmd(cmd_helper_cat128, "cat128");
 
 	x->cat = 15;
@@ -528,14 +492,12 @@ cmd_helper_cat128(void)
 void
 cat(unsigned long nff)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	unsigned long p;
 	unsigned long ff;
 
-	f = &x->f;
-
 	p = 0;
 	ff = 0;
 
@@ -575,22 +537,15 @@ void
 check_cmd(void (*fn)(void),
     const char *name)
 {
-	struct xstate *x = xstatus();
-	unsigned long i;
+	struct xstate *x = xstatus(0, NULL);
+	unsigned long i = x->i;
 
-	if (x->cmd[x->i].run != fn)
+	if (x->cmd[i].run != fn)
 		err(ECANCELED, "Running %s, but cmd %s is set",
-		    name, x->cmd[x->i].str);
+		    name, x->cmd[i].str);
 
-	/*
-	 * In addition to making sure we ran
-	 * the right command, we now disable
-	 * all commands from running again
-	 *
-	 * the _nop function will just call
-	 * err() immediately
+	/* prevent second command
 	 */
-
 	for (i = 0; i < items(x->cmd); i++)
 		x->cmd[i].run = cmd_helper_err;
 }

util/nvmutil/lib/file.c

@@ -24,21 +24,40 @@
 
 #include "../include/common.h"
 
-/*
- * TODO: make generic. S_ISREG: check every other
- * type, erring only if it doesn't match what was
- * passed as type requested.
- * also:
- * have variable need_seek, only err on seek if
- * need_seek is set.
- * also consider the stat check in this generic
- * context
- * make tthe return type an int, not a void.
- * return -1 with errno set to indicate error,
- * though the syscalls mostly handle that.
- * save errno before lseek, resetting it after
- * the check if return >-1
+/* check that a file changed
  */
+
+int
+same_file(int fd, struct stat *st_old,
+    int check_size)
+{
+	struct stat st;
+	int saved_errno = errno;
+
+	if (st_old == NULL || fd < 0)
+		goto err_same_file;
+
+	if (fstat(fd, &st) == -1)
+		return -1;
+
+	if (st.st_dev != st_old->st_dev ||
+	    st.st_ino != st_old->st_ino ||
+	    !S_ISREG(st.st_mode))
+		goto err_same_file;
+
+	if (check_size &&
+	    st.st_size != st_old->st_size)
+		goto err_same_file;
+
+	errno = saved_errno;
+	return 0;
+
+err_same_file:
+
+	errno = EIO;
+	return -1;
+}
+
 void
 xopen(int *fd_ptr, const char *path, int flags, struct stat *st)
 {
@@ -55,10 +74,10 @@ xopen(int *fd_ptr, const char *path, int flags, struct stat *st)
 		err(errno, "%s: file not seekable", path);
 }
 
-/*
- * Ensure rename() is durable by syncing the
+/* Ensure x_i_rename() is durable by syncing the
  * directory containing the target file.
  */
+
 int
 fsync_dir(const char *path)
 {
@@ -165,8 +184,7 @@ err_fsync_dir:
 	return -1;
 }
 
-/*
- * create new tmpfile path
+/* create new tmpfile path
  *
  * ON SUCCESS:
  *
@@ -189,14 +207,14 @@ err_fsync_dir:
  * if local is zero, then 3rd arg (path)
  * is irrelevant and can be NULL
  */
+
 char *
 new_tmpfile(int *fd, int local, const char *path)
 {
 	unsigned long maxlen;
 	struct stat st;
 
-	/*
-	 * please do not modify the
+	/* please do not modify the
 	 * strings or I will get mad
 	 */
 	char tmp_none[] = "";
@@ -250,8 +268,7 @@ new_tmpfile(int *fd, int local, const char *path)
 	if (local) {
 		base = tmp_none;
 
-		/*
-		 * appended to filename for tmp:
+		/* appended to filename for tmp:
 		 */
 		tmpdir_len = xstrxlen(default_tmpname, maxlen);
 	} else {
@@ -270,8 +287,7 @@ new_tmpfile(int *fd, int local, const char *path)
 	tmppath_len = tmpdir_len + tmpname_len;
 	++tmppath_len; /* for '/' or '.' */
 
-	/*
-	 * max length -1 of maxlen
+	/* max length -1 of maxlen
 	 * for termination
 	 */
 	if (tmpdir_len > maxlen - tmpname_len - 1)
@@ -389,7 +405,6 @@ x_c_tmpdir(void)
 	char *t;
 	struct stat st;
 
-	t = getenv("TMPDIR");
 	t = getenv("TMPDIR");
 
 	if (t && *t) {
@@ -409,9 +424,9 @@ x_c_tmpdir(void)
 	return ".";
 }
 
-/*
- * portable mkstemp
+/* portable mkstemp
  */
+
 int
 x_i_mkstemp(char *template)
 {
@@ -436,7 +451,9 @@ x_i_mkstemp(char *template)
 	for (i = 0; i < 100; i++) {
 
 		for (j = 0; j < 6; j++) {
+
 			r = rlong();
+
 			p[j] = ch[(unsigned long)(r >> 1) % (sizeof(ch) - 1)];
 		}
 
@@ -466,8 +483,7 @@ x_i_mkstemp(char *template)
  * EINTR/EAGAIN looping is done indefinitely.
  */
 
-/*
- * rw_file_exact() - Read perfectly or die
+/* rw_file_exact() - Read perfectly or die
  *
  * Read/write, and absolutely insist on an
  * absolute read; e.g. if 100 bytes are
@@ -483,6 +499,7 @@ x_i_mkstemp(char *template)
  * times upon zero-return, to recover,
  * otherwise it will return an error.
  */
+
 long
 rw_file_exact(int fd, unsigned char *mem, unsigned long nrw,
     off_t off, int rw_type, int loop_eagain,
@@ -549,8 +566,7 @@ err_rw_file_exact:
 	return -1;
 }
 
-/*
- * prw() - portable read-write
+/* prw() - portable read-write
  *
  * This implements a portable analog of pwrite()
  * and pread() - note that this version is not
@@ -809,19 +825,17 @@ err_is_file:
 	return -1;
 }
 
-/*
- * Check overflows caused by buggy libc.
+/* Check weirdness on buggy libc.
  *
  * POSIX can say whatever it wants.
  * specification != implementation
  */
+
 long
 rw_over_nrw(long r, unsigned long nrw)
 {
-	/*
-	 * If a byte length of zero
-	 * was requested, that is
-	 * clearly a bug. No way.
+	/* not a libc bug, but we
+	 * don't like the number zero
 	 */
 	if (!nrw)
 		goto err_rw_over_nrw;
@@ -832,8 +846,7 @@ rw_over_nrw(long r, unsigned long nrw)
 	if ((unsigned long)
 	    r > X_LONG_MAX) {
 
-		/*
-		 * Theoretical buggy libc
+		/* Theoretical buggy libc
 		 * check. Extremely academic.
 		 *
 		 * Specifications never
@@ -843,12 +856,16 @@ rw_over_nrw(long r, unsigned long nrw)
 		 *
 		 * Check this after using
 		 * [p]read() or [p]write()
+		 *
+		 * NOTE: here, we assume
+		 * long integers are the
+		 * same size as SSIZE_T
 		 */
+
 		goto err_rw_over_nrw;
 	}
 
-	/*
-	 * Theoretical buggy libc:
+	/* Theoretical buggy libc:
 	 * Should never return a number of
 	 * bytes above the requested length.
 	 */
@@ -888,35 +905,31 @@ lseek_loop(int fd, off_t off, int whence,
 }
 #endif
 
-/*
- * If a given error loop is enabled,
- * e.g. EINTR or EAGAIN, an I/O operation
- * will loop until errno isn't -1 and one
- * of these, e.g. -1 and EINTR
- */
 int
 try_err(int loop_err, int errval)
 {
 	if (loop_err)
 		return errval;
 
-	/* errno is never negative,
-	   so functions checking it
-	   can use it accordingly */
 	return -1;
 }
 
-/*
- * non-atomic rename
+/* portable rename(). WARNING:
+ * not powercut-safe. do this to
+ * use system rename:
+ * #define SYS_RENAME 1
  *
- * commented because i can't sacrifice
- * exactly this property. nvmutil tries
- * to protect files against e.g. power loss
+ * written academically, but in reality,
+ * nearly all unix systems have rename()
  */
-/*
+
 int
 x_i_rename(const char *src, const char *dst)
 {
+#if defined(SYS_RENAME) &&\
+    SYS_RENAME > 0
+	return rename(src, dst);
+#else
 	int sfd, dirfd;
 	ssize_t r;
 	char buf[8192];
@@ -955,8 +968,8 @@ x_i_rename(const char *src, const char *dst)
 		return -1;
 
 	return 0;
+#endif
 }
-*/
 
 int
 x_i_close(int fd)

util/nvmutil/lib/io.c

@@ -3,8 +3,6 @@
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
  *
  * I/O functions specific to nvmutil.
- *
- * Related: file.c
  */
 
 #ifdef __OpenBSD__
@@ -29,30 +27,22 @@
 void
 open_gbe_file(void)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
-	struct stat _st;
 	int _flags;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	xopen(&f->gbe_fd, f->fname,
 	    cmd->flags | O_BINARY |
-	    O_NOFOLLOW | O_CLOEXEC, &_st);
+	    O_NOFOLLOW | O_CLOEXEC, &f->gbe_st);
 
-	/* inode will be checked later on write */
-	f->gbe_dev = _st.st_dev;
-	f->gbe_ino = _st.st_ino;
-
-	if (_st.st_nlink > 1)
+	if (f->gbe_st.st_nlink > 1)
 		err(EINVAL,
 		    "%s: warning: file has multiple (%lu) hard links\n",
-		    f->fname, (unsigned long)_st.st_nlink);
+		    f->fname, (unsigned long)f->gbe_st.st_nlink);
 
-	if (_st.st_nlink == 0)
+	if (f->gbe_st.st_nlink == 0)
 		err(EIO, "%s: file unlinked while open", f->fname);
 
 	_flags = fcntl(f->gbe_fd, F_GETFL);
@@ -68,7 +58,7 @@ open_gbe_file(void)
 	if (_flags & O_APPEND)
 		err(EIO, "%s: O_APPEND flag", f->fname);
 
-	f->gbe_file_size = _st.st_size;
+	f->gbe_file_size = f->gbe_st.st_size;
 
 	switch (f->gbe_file_size) {
 	case SIZE_8KB:
@@ -83,37 +73,14 @@ open_gbe_file(void)
 		err(errno, "%s: can't lock", f->fname);
 }
 
-/*
- * We copy the entire gbe file
- * to the tmpfile, and then we
- * work on that. We copy back
- * afterward. this is the copy.
- *
- * we copy to tmpfile even on
- * read-only commands, for the
- * double-read verification,
- * which also benefits cmd_cat.
- */
 void
 copy_gbe(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
-
-	f = &x->f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	read_file();
 
-	/*
-	 regular operations post-read operate only on the first
-	 8KB, because each GbE part is the first 4KB of each
-	 half of the file.
-
-	 we no longer care about anything past 8KB, until we get
-	 to writing, at which point we will flush the buffer
-	 again
-	 */
-
 	if (f->gbe_file_size == SIZE_8KB)
 		return;
 
@@ -125,15 +92,14 @@ copy_gbe(void)
 void
 read_file(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	struct stat _st;
 	long _r;
 
-	f = &x->f;
-
-	/* read main file */
+	/* read main file
+	 */
 	_r = rw_file_exact(f->gbe_fd, f->buf, f->gbe_file_size,
 	    0, IO_PREAD, NO_LOOP_EAGAIN, LOOP_EINTR,
 	    MAX_ZERO_RW_RETRY, OFF_ERR);
@@ -141,8 +107,8 @@ read_file(void)
 	if (_r < 0)
 		err(errno, "%s: read failed", f->fname);
 
-	/* copy to tmpfile */
-
+	/* copy to tmpfile
+	 */
 	_r = rw_file_exact(f->tmp_fd, f->buf, f->gbe_file_size,
 	    0, IO_PWRITE, NO_LOOP_EAGAIN, LOOP_EINTR,
 	    MAX_ZERO_RW_RETRY, OFF_ERR);
@@ -151,10 +117,8 @@ read_file(void)
 		err(errno, "%s: %s: copy failed",
 		    f->fname, f->tname);
 
-	/*
-	 * file size comparison
+	/* file size comparison
 	 */
-
 	if (fstat(f->tmp_fd, &_st) == -1)
 		err(errno, "%s: stat", f->tname);
 
@@ -164,9 +128,7 @@ read_file(void)
 		err(EIO, "%s: %s: not the same size",
 		    f->fname, f->tname);
 
-	/*
-	 * fsync tmp gbe file, because we will compare
-	 * its contents to what was read (for safety)
+	/* needs sync, for verification
 	 */
 	if (x_i_fsync(f->tmp_fd) == -1)
 		err(errno, "%s: fsync (tmpfile copy)", f->tname);
@@ -182,42 +144,25 @@ read_file(void)
 		err(errno, "%s: %s: read contents differ (pre-test)",
 		    f->fname, f->tname);
 }
+
 void
 write_gbe_file(void)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
-
-	struct stat _gbe_st;
-	struct stat _tmp_st;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
 	unsigned long p;
 	unsigned char update_checksum;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	if ((cmd->flags & O_ACCMODE) == O_RDONLY)
 		return;
 
-	if (fstat(f->gbe_fd, &_gbe_st) == -1)
-		err(errno, "%s: re-check", f->fname);
-	if (_gbe_st.st_dev != f->gbe_dev || _gbe_st.st_ino != f->gbe_ino)
-		err(EIO, "%s: file replaced while open", f->fname);
-	if (_gbe_st.st_size != f->gbe_file_size)
-		err(errno, "%s: file size changed before write", f->fname);
-	if (!S_ISREG(_gbe_st.st_mode))
-		err(errno, "%s: file type changed before write", f->fname);
+	if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0)
+		err(errno, "%s: file inode/device changed", f->tname);
 
-	if (fstat(f->tmp_fd, &_tmp_st) == -1)
-		err(errno, "%s: re-check", f->tname);
-	if (_tmp_st.st_dev != f->tmp_dev || _tmp_st.st_ino != f->tmp_ino)
-		err(EIO, "%s: file replaced while open", f->tname);
-	if (_tmp_st.st_size != f->gbe_file_size)
-		err(errno, "%s: file size changed before write", f->tname);
-	if (!S_ISREG(_tmp_st.st_mode))
-		err(errno, "%s: file type changed before write", f->tname);
+	if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0)
+		err(errno, "%s: file has changed", f->fname);
 
 	update_checksum = cmd->chksum_write;
 
@@ -236,9 +181,9 @@ void
 rw_gbe_file_part(unsigned long p, int rw_type,
     const char *rw_type_str)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
 	long rval;
 
@@ -247,9 +192,6 @@ rw_gbe_file_part(unsigned long p, int rw_type,
 	unsigned long gbe_rw_size;
 	unsigned char *mem_offset;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	gbe_rw_size = cmd->rw_size;
 
 	if (rw_type < IO_PREAD || rw_type > IO_PWRITE)
@@ -274,16 +216,13 @@ rw_gbe_file_part(unsigned long p, int rw_type,
 void
 write_to_gbe_bin(void)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
 	int saved_errno;
 	int mv;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	if ((cmd->flags & O_ACCMODE) != O_RDWR)
 		return;
 
@@ -338,11 +277,9 @@ write_to_gbe_bin(void)
 			fprintf(stderr, "%s: %s\n",
 			    f->fname, strerror(errno));
 		} else {
-			/*
-			 * tmpfile removed
-			 * by the rename
-			 */
 
+			/* removed by rename
+			 */
 			if (f->tname != NULL)
 				free(f->tname);
 
@@ -350,12 +287,6 @@ write_to_gbe_bin(void)
 		}
 	}
 
-	/*
-	 * finally:
-	 * must sync to disk!
-	 * very nearly done
-	 */
-
 	if (!f->io_err_gbe_bin)
 		return;
 
@@ -369,9 +300,9 @@ write_to_gbe_bin(void)
 void
 check_written_part(unsigned long p)
 {
-	struct xstate *x = xstatus();
-	struct commands *cmd;
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
 	long rval;
 
@@ -380,12 +311,8 @@ check_written_part(unsigned long p)
 	off_t file_offset;
 	unsigned char *mem_offset;
 
-	struct stat st;
 	unsigned char *buf_restore;
 
-	cmd = &x->cmd[x->i];
-	f = &x->f;
-
 	if (!f->part_modified[p])
 		return;
 
@@ -396,15 +323,11 @@ check_written_part(unsigned long p)
 
 	memset(f->pad, 0xff, sizeof(f->pad));
 
-	if (fstat(f->gbe_fd, &st) == -1)
-		err(errno, "%s: fstat (post-write)", f->fname);
-	if (st.st_dev != f->gbe_dev || st.st_ino != f->gbe_ino)
-		err(EIO, "%s: file changed during write", f->fname);
+	if (same_file(f->tmp_fd, &f->tmp_st, 0) < 0)
+		err(errno, "%s: file inode/device changed", f->tname);
 
-	if (fstat(f->tmp_fd, &st) == -1)
-		err(errno, "%s: fstat (post-write)", f->tname);
-	if (st.st_dev != f->tmp_dev || st.st_ino != f->tmp_ino)
-		err(EIO, "%s: file changed during write", f->tname);
+	if (same_file(f->gbe_fd, &f->gbe_st, 1) < 0)
+		err(errno, "%s: file changed during write", f->fname);
 
 	rval = rw_gbe_file_exact(f->tmp_fd, f->pad,
 	    gbe_rw_size, file_offset, IO_PREAD);
@@ -442,13 +365,11 @@ check_written_part(unsigned long p)
 void
 report_io_err_rw(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	unsigned long p;
 
-	f = &x->f;
-
 	if (!f->io_err_gbe)
 		return;
 
@@ -500,8 +421,8 @@ report_io_err_rw(void)
 int
 gbe_mv(void)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	int rval;
 
@@ -511,8 +432,6 @@ gbe_mv(void)
 	char *dest_tmp;
 	int dest_fd;
 
-	f = &x->f;
-
 	/* will be set 0 if it doesn't */
 	tmp_gbe_bin_exists = 1;
 
@@ -521,7 +440,7 @@ gbe_mv(void)
 
 	saved_errno = errno;
 
-	rval = rename(f->tname, f->fname);
+	rval = x_i_rename(f->tname, f->fname);
 
 	if (rval > -1) {
 		/*
@@ -576,7 +495,7 @@ gbe_mv(void)
 	if (x_i_close(dest_fd) == -1)
 		goto ret_gbe_mv;
 
-	if (rename(dest_tmp, f->fname) == -1)
+	if (x_i_rename(dest_tmp, f->fname) == -1)
 		goto ret_gbe_mv;
 
 	if (fsync_dir(f->fname) < 0) {
@@ -640,13 +559,11 @@ ret_gbe_mv:
 unsigned char *
 gbe_mem_offset(unsigned long p, const char *f_op)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	off_t gbe_off;
 
-	f = &x->f;
-
 	gbe_off = gbe_x_offset(p, f_op, "mem",
 	    GBE_PART_SIZE, GBE_WORK_SIZE);
 
@@ -664,13 +581,11 @@ gbe_mem_offset(unsigned long p, const char *f_op)
 off_t
 gbe_file_offset(unsigned long p, const char *f_op)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	off_t gbe_file_half_size;
 
-	f = &x->f;
-
 	gbe_file_half_size = f->gbe_file_size >> 1;
 
 	return gbe_x_offset(p, f_op, "file",
@@ -681,15 +596,13 @@ off_t
 gbe_x_offset(unsigned long p, const char *f_op, const char *d_type,
     off_t nsize, off_t ncmp)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	off_t off;
 
 	check_bin(p, "part number");
 
-	f = &x->f;
-
 	off = ((off_t)p) * (off_t)nsize;
 
 	if (off > ncmp - GBE_PART_SIZE)
@@ -707,13 +620,11 @@ long
 rw_gbe_file_exact(int fd, unsigned char *mem, unsigned long nrw,
     off_t off, int rw_type)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	long r;
 
-	f = &x->f;
-
 	if (io_args(fd, mem, nrw, off, rw_type) == -1)
 		return -1;
 

util/nvmutil/lib/num.c

@@ -45,9 +45,7 @@ hextonum(char ch_s)
 	return 16; /* invalid character */
 }
 
-/*
- * Portable random
- * number generator
+/* Random numbers
  */
 unsigned long
 rlong(void)

util/nvmutil/lib/state.c

@@ -5,13 +5,6 @@
  * This tool lets you modify Intel GbE NVM (Gigabit Ethernet
  * Non-Volatile Memory) images, e.g. change the MAC address.
  * These images configure your Intel Gigabit Ethernet adapter.
- *
- * This code is designed to be portable, running on as many
- * Unix and Unix-like systems as possible (mainly BSD/Linux).
- *
- * Recommended CFLAGS for Clang/GCC:
- *
- * -Os -Wall -Wextra -Werror -pedantic -std=c90
  */
 
 #ifdef __OpenBSD__
@@ -34,40 +27,16 @@
 #include "../include/common.h"
 
 /*
- * Program state/command table
- * Default config stored here,
- * and copied to a newly allocated
- * buffer in memory, then the pointer
- * is passed. The rest of the program
- * will manipulate this data.
- */
-/*
-TODO: 
-eventually, i will not have this return
-a pointer at all. instead, a similar key
-mechanism will be used for other access
-functions e.g. word/set_word, err(needs
-to clean up), and so on. then those
-would return values if already initialised,
-but would not permit additional init - will
-decide exactly how to implement this at a
-later state.
-
-this is part of an ongoing effort to introduce
-extreme memory safety into this program.
+ * Initialise program state,
+ * load GbE file and verify
+ * data, ready for operation
+ * (singleton design)
  */
 struct xstate *
-xstatus(void)
+xstatus(int argc, char *argv[])
 {
-	static int first_run = 1;
-
 	static struct xstate us = {
-	/* .cmd (update cmd[] in the struct if adding to it)
-	         DO NOT FORGET. or C will init zeroes/NULLs */
-
-	/* cmd[] members */
-		/* DO NOT MESS THIS UP */
-		/* items must be set *exactly* */
+	/* DO NOT MESS THIS UP, OR THERE WILL BE DEMONS */
 	{
 	{
 		CMD_DUMP, "dump", cmd_helper_dump, ARGC_3,
@@ -122,36 +91,165 @@ xstatus(void)
 	/* .no_cmd (set 0 when a command is found) */
 	1,
 
-	/* .xsize (size of the stuct will be stored here later) */
-	0,
-
 	/* .cat (cat helpers set this) */
 	-1
 
 	};
 
+	static int first_run = 1;
+
 	if (!first_run)
 		return &us;
 
-	first_run = 0;
-
-	us.xsize = sizeof(us);
-
 	us.f.buf = us.f.real_buf;
 
-	us.f.gbe_fd = -1;
-	us.f.tmp_fd = -1;
+	first_run = 0;
+	us.argv0 = argv[0];
 
-	us.f.tname = NULL;
-	us.f.fname = NULL;
+	if (argc > 1)
+		us.f.fname = argv[1];
+
+	if (argc < 3)
+		usage();
+
+/* https://man.openbsd.org/pledge.2
+   https://man.openbsd.org/unveil.2     */
+#if defined(__OpenBSD__) && defined(OpenBSD)
+#if (OpenBSD) >= 604
+	if (pledge("stdio flock rpath wpath cpath unveil", NULL) == -1)
+		err(errno, "pledge plus unveil");
+#elif (OpenBSD) >= 509
+	if (pledge("stdio flock rpath wpath cpath", NULL) == -1)
+		err(errno, "pledge");
+#endif
+#endif
+
+#ifndef S_ISREG
+	err(ECANCELED, "Can't determine file types (S_ISREG undefined)");
+#endif
+
+#ifndef CHAR_BIT
+	err(ECANCELED, "Unknown char size");
+#else
+	if (CHAR_BIT != 8)
+		err(EINVAL, "Unsupported char size");
+#endif
+
+#if defined(__OpenBSD__) && defined(OpenBSD) && \
+    (OpenBSD) >= 604
+	/* can only use local tmp on openbsd, due to unveil */
+	us.f.tname = new_tmpfile(&us.f.tmp_fd, 1, NULL);
+#else
+	us.f.tname = new_tmpfile(&us.f.tmp_fd, 0, NULL);
+#endif
+	if (us.f.tname == NULL)
+		err(errno, "Can't create tmpfile");
+	if (*us.f.tname == '\0')
+		err(errno, "tmp dir is an empty string");
+
+#if defined(__OpenBSD__) && defined(OpenBSD) && \
+    OpenBSD >= 604
+	if (unveil(f->tname, "rwc") == -1)
+		err(errno, "unveil rwc: %s", f->tname);
+#endif
+	if (fstat(us.f.tmp_fd, &us.f.tmp_st) < 0)
+		err(errno, "%s: stat", us.f.tname);
+
+	sanitize_command_list();
+
+	/* parse user command */
+	set_cmd(argc, argv);
+	set_cmd_args(argc, argv);
+
+#if defined(__OpenBSD__) && defined(OpenBSD) && \
+    (OpenBSD) >= 604
+	if ((us.cmd[i].flags & O_ACCMODE) == O_RDONLY) {
+		if (unveil(us.f.fname, "r") == -1)
+			err(errno, "%s: unveil r", us.f.fname);
+	} else {
+		if (unveil(us.f.fname, "rwc") == -1)
+			err(errno, "%s: unveil rw", us.f.tname);
+	}
+
+	if (unveil(us.f.tname, "rwc") == -1)
+		err(errno, "%s: unveil rwc", us.f.tname);
+
+	if (unveil(NULL, NULL) == -1)
+		err(errno, "unveil block (rw)");
+
+	if (pledge("stdio flock rpath wpath cpath", NULL) == -1)
+		err(errno, "pledge (kill unveil)");
+#endif
+
+	open_gbe_file();
+
+	memset(us.f.real_buf, 0, sizeof(us.f.real_buf));
+	memset(us.f.bufcmp, 0, sizeof(us.f.bufcmp));
+
+	/* for good measure */
+	memset(us.f.pad, 0, sizeof(us.f.pad));
+
+	copy_gbe();
+	read_checksums();
 
 	return &us;
 }
 
+void
+err(int nvm_errval, const char *msg, ...)
+{
+	struct xstate *x = xstatus(0, NULL);
+
+	va_list args;
+
+	if (errno == 0)
+		errno = nvm_errval;
+	if (!errno)
+		errno = ECANCELED;
+
+	(void)exit_cleanup();
+
+	if (x != NULL)
+		fprintf(stderr, "%s: ", getnvmprogname());
+
+	va_start(args, msg);
+	vfprintf(stderr, msg, args);
+	va_end(args);
+
+	fprintf(stderr, ": %s\n", strerror(errno));
+
+	exit(EXIT_FAILURE);
+}
+
+const char *
+getnvmprogname(void)
+{
+	struct xstate *x = xstatus(0, NULL);
+
+	const char *p;
+	static char fallback[] = "nvmutil";
+
+	char *rval = fallback;
+
+	if (x != NULL) {
+		if (x->argv0 == NULL || *x->argv0 == '\0')
+			return "";
+
+		rval = x->argv0;
+	}
+
+	p = x_c_strrchr(rval, '/');
+
+	if (p)
+		return p + 1;
+	else
+		return rval;
+}
+
 int
 exit_cleanup(void)
 {
-	struct xstate *x = xstatus();
+	struct xstate *x = xstatus(0, NULL);
 	struct xfile *f;
 
 	int close_err;

util/nvmutil/lib/string.c

@@ -93,67 +93,14 @@ xstrxlen(const char *scmp, unsigned long maxlen)
 	return xstr_index;
 }
 
-void
-err(int nvm_errval, const char *msg, ...)
-{
-	struct xstate *x = xstatus();
-
-	va_list args;
-
-	if (errno == 0)
-		errno = nvm_errval;
-	if (!errno)
-		errno = ECANCELED;
-
-	(void)exit_cleanup();
-
-	if (x != NULL)
-		fprintf(stderr, "%s: ", getnvmprogname());
-
-	va_start(args, msg);
-	vfprintf(stderr, msg, args);
-	va_end(args);
-
-	fprintf(stderr, ": %s\n", strerror(errno));
-
-	exit(EXIT_FAILURE);
-}
-
-const char *
-getnvmprogname(void)
-{
-	struct xstate *x = xstatus();
-
-	const char *p;
-	static char fallback[] = "nvmutil";
-
-	char *rval = fallback;
-
-	if (x != NULL) {
-		if (x->argv0 == NULL || *x->argv0 == '\0')
-			return "";
-
-		rval = x->argv0;
-	}
-
-	p = x_c_strrchr(rval, '/');
-
-	if (p)
-		return p + 1;
-	else
-		return rval;
-}
-
 char *
 x_c_strrchr(const char *s, int c)
 {
 	const char *p = NULL;
 
-	while (*s) {
+	for ( ; *s; s++)
 		if (*s == (char)c)
 			p = s;
-		s++;
-	}
 
 	if (c == '\0')
 		return (char *)s;

util/nvmutil/lib/word.c

@@ -29,20 +29,17 @@
  * GbE NVM files store 16-bit (2-byte) little-endian words.
  * We must therefore swap the order when reading or writing.
  *
- * NOTE: The MAC address words are stored big-endian in the
- * file, but we assume otherwise and adapt accordingly.
+ * NOTE: The MAC address words are stored big-endian in-file.
  */
 
 unsigned short
 nvm_word(unsigned long pos16, unsigned long p)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	unsigned long pos;
 
-	f = &x->f;
-
 	check_nvm_bound(pos16, p);
 	pos = (pos16 << 1) + (p * GBE_PART_SIZE);
 
@@ -53,13 +50,11 @@ nvm_word(unsigned long pos16, unsigned long p)
 void
 set_nvm_word(unsigned long pos16, unsigned long p, unsigned short val16)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	unsigned long pos;
 
-	f = &x->f;
-
 	check_nvm_bound(pos16, p);
 	pos = (pos16 << 1) + (p * GBE_PART_SIZE);
 
@@ -72,10 +67,8 @@ set_nvm_word(unsigned long pos16, unsigned long p, unsigned short val16)
 void
 set_part_modified(unsigned long p)
 {
-	struct xstate *x = xstatus();
-	struct xfile *f;
-
-	f = &x->f;
+	struct xstate *x = xstatus(0, NULL);
+	struct xfile *f = &x->f;
 
 	check_bin(p, "part number");
 	f->part_modified[p] = 1;
@@ -84,10 +77,7 @@ set_part_modified(unsigned long p)
 void
 check_nvm_bound(unsigned long c, unsigned long p)
 {
-	/*
-	 * NVM_SIZE assumed as the limit, because this
-	 * current design assumes that we will only
-	 * ever modified the NVM area.
+	/* Block out of bound NVM access
 	 */
 
 	check_bin(p, "part number");