util/nvmutil: tidy up includes

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

util/nvmutil/include/common.h

@@ -4,26 +4,37 @@
  * Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
  */
 
-/*
- * TODO: split this per .c file
- */
-
-/* Use this shorthand in cmd helpers. e.g.
-   in cmd_setmac function:
-   check_cmd(cmd_helper_cat);
-*/
-
 #ifndef COMMON_H
 #define COMMON_H
 
-/*
- * system prototypes
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <limits.h>
+
+/* keep SYS_RENAME 1 to
+ * use libc rename()
+ * recommended
+ */
+#ifndef SYS_RENAME
+#define SYS_RENAME 1
+#endif
+
+#define items(x) (sizeof((x)) / sizeof((x)[0]))
+
+/* system prototypes
  */
 
 int fchmod(int fd, mode_t mode);
 
-/*
- * build config
+/* analog of SSIZE_MAX
+ */
+
+#ifndef X_LONG_MAX
+#define X_LONG_MAX ((long)(~((long)1 << (sizeof(long)*CHAR_BIT-1))))
+#endif
+
+
+/* build config
  */
 
 #ifndef NVMUTIL_H
@@ -71,27 +82,6 @@ int fchmod(int fd, mode_t mode);
 #define _FILE_OFFSET_BITS 64
 #endif
 
-/*
- * Older versions of BSD to the early 2000s
- * could compile nvmutil, but pledge was
- * added in the 2010s. Therefore, for extra
- * portability, we will only pledge/unveil
- * on OpenBSD versions that have it.
- */
-
-#if defined(__OpenBSD__) && defined(OpenBSD)
-#if OpenBSD >= 604
-#ifndef NVMUTIL_UNVEIL
-#define NVMUTIL_UNVEIL 1
-#endif
-#endif
-#if OpenBSD >= 509
-#ifndef NVMUTIL_PLEDGE
-#define NVMUTIL_PLEDGE 1
-#endif
-#endif
-#endif
-
 #ifndef EXIT_FAILURE
 #define EXIT_FAILURE 1
 #endif
@@ -132,8 +122,7 @@ int fchmod(int fd, mode_t mode);
 #define FD_CLOEXEC 0
 #endif
 
-/*
- * Sizes in bytes:
+/* Sizes in bytes:
  */
 
 #define SIZE_1KB 1024
@@ -144,17 +133,12 @@ int fchmod(int fd, mode_t mode);
 
 #define GBE_BUF_SIZE (SIZE_128KB)
 
-/*
- * First 128 bytes of a GbE part contains
- * the regular NVM (Non-Volatile-Memory)
- * area. All of these bytes must add up,
- * truncated to 0xBABA.
+/* First 128 bytes of gbe.bin is NVM.
+ * Then extended area. All of NVM must
+ * add up to BABA, truncated (LE)
  *
- * The full GbE region is 4KB, but only
- * the first 128 bytes are used here.
- *
- * There is a second 4KB part with the same
- * rules, and it *should* be identical.
+ * First 4KB of each half of the file
+ * contains NVM+extended.
  */
 
 #define GBE_WORK_SIZE (SIZE_8KB)
@@ -164,29 +148,7 @@ int fchmod(int fd, mode_t mode);
 #define NVM_WORDS (NVM_SIZE >> 1)
 #define NVM_CHECKSUM_WORD (NVM_WORDS - 1)
 
-/*
- * Portable macro based on BSD nitems.
- * Used to count the number of commands (see below).
- */
-
-#define items(x) (sizeof((x)) / sizeof((x)[0]))
-
-/*
- * GbE files can be 8KB, 16KB or 128KB,
- * but we only need the two 4KB parts
- * from offset zero and offset 64KB in
- * a 128KB file, or zero and 8KB in a 16KB
- * file, or zero and 4KB in an 8KB file.
- *
- * The code will handle this properly.
- */
-
-#ifndef X_LONG_MAX
-#define X_LONG_MAX ((long)(~((long)1 << (sizeof(long)*CHAR_BIT-1))))
-#endif
-
-/*
- * Use these for .argc in command[]:
+/* argc minimum (dispatch)
  */
 
 #define ARGC_3 3
@@ -195,16 +157,11 @@ int fchmod(int fd, mode_t mode);
 #define NO_LOOP_EAGAIN 0
 #define NO_LOOP_EINTR 0
 
-/*
- * Used for checking whether.
- * a file is a file via stat().
+/* For checking if an fd is a normal file.
+ * Portable for old Unix e.g. v7 (S_IFREG),
+ * 4.2BSD (S_IFMT), POSIX (S_ISREG).
  *
- * Portable macro for compatibility
- * with older unix e.g. v7 unix (has S_IFREG),
- * 4.2bsd (has S_IFMT) or POSIX (has S_ISREG)
- *
- * Fallback works where S_IFREG == 0100000
- * (classic unix bitmask)
+ * IFREG: assumed 0100000 (classic bitmask)
  */
 
 #ifndef S_ISREG
@@ -222,9 +179,7 @@ int fchmod(int fd, mode_t mode);
 #define IO_PREAD 2
 #define IO_PWRITE 3
 
-/*
- * Used as indices for command[]
- * MUST be in the same order as entries in command[]
+/* for nvmutil commands
  */
 
 #define CMD_DUMP 0
@@ -244,6 +199,9 @@ int fchmod(int fd, mode_t mode);
 #define SKIP_CHECKSUM_WRITE 0
 #define CHECKSUM_WRITE 1
 
+/* command table
+ */
+
 struct commands {
 	unsigned long chk;
 	char *str;
@@ -256,15 +214,24 @@ struct commands {
 	int flags; /* e.g. O_RDWR or O_RDONLY */
 };
 
+/* mac address
+ */
+
 struct macaddr {
 	char *str; /* set to rmac, or argv string */
 	char rmac[18]; /* xx:xx:xx:xx:xx:xx */
 	unsigned short mac_buf[3];
 };
 
+/* gbe.bin and tmpfile
+ */
+
 struct xfile {
 	int gbe_fd;
+	struct stat gbe_st;
+
 	int tmp_fd;
+	struct stat tmp_st;
 
 	char *tname; /* path of tmp file */
 	char *fname; /* path of gbe file */
@@ -279,12 +246,6 @@ struct xfile {
 
 	int post_rw_checksum[2];
 
-	dev_t gbe_dev;
-	ino_t gbe_ino;
-
-	dev_t tmp_dev;
-	ino_t tmp_ino;
-
 	off_t gbe_file_size;
 	off_t gbe_tmp_size;
 
@@ -298,11 +259,13 @@ struct xfile {
 	unsigned char pad[GBE_WORK_SIZE]; /* the file that wouldn't die */
 };
 
-/*
+/* Command table, MAC address, files
+ *
  * BE CAREFUL when editing this
  * to ensure that you also update
  * the tables in xstatus()
  */
+
 struct xstate {
 	struct commands cmd[7];
 	struct macaddr mac;
@@ -313,10 +276,6 @@ struct xstate {
 	unsigned long i; /* index to cmd[] for current command */
 	int no_cmd;
 
-	/* store size of a struct here.
-	   (can be used to copy old state) */
-	unsigned long xsize;
-
 	/* Cat commands set this.
 	   the cat cmd helpers check it */
 	int cat;
@@ -324,52 +283,47 @@ struct xstate {
 
 
 
+struct xstate *xstatus(int argc, char *argv[]);
 
-struct xstate *xstatus(void);
-
-/*
- * Sanitize command tables.
+/* Sanitize command tables.
  */
+
 void sanitize_command_list(void);
 void sanitize_command_index(unsigned long c);
 
-/*
- * Argument handling (user input)
+/* Argument handling (user input)
  */
+
 void set_cmd(int argc, char *argv[]);
 void set_cmd_args(int argc, char *argv[]);
 unsigned long conv_argv_part_num(const char *part_str);
 int xstrxcmp(const char *a, const char *b, unsigned long maxlen);
 
-/*
- * Prep files for reading
+/* Prep files for reading
  */
+
 void open_gbe_file(void);
 int lock_file(int fd, int flags);
+int same_file(int fd, struct stat *st_old, int check_size);
 void xopen(int *fd, const char *path, int flags, struct stat *st);
 
-/*
- * Read GbE file and verify
- * checksums.
- *
- * After this, we can run commands.
+/* Read GbE file and verify checksums
  */
+
 void copy_gbe(void);
 void read_file(void);
 void read_checksums(void);
 int good_checksum(unsigned long partnum);
 
-/*
- * Execute user command on GbE data.
- * These are stubs that call helpers.
+/* validate commands
  */
-void run_cmd(void);
+
 void check_command_num(unsigned long c);
 unsigned char valid_command(unsigned long c);
 
-/*
- * Helper functions for command: setmac
+/* Helper functions for command: setmac
  */
+
 void cmd_helper_setmac(void);
 void parse_mac_string(void);
 unsigned long xstrxlen(const char *scmp, unsigned long maxlen);
@@ -380,51 +334,49 @@ unsigned short hextonum(char ch_s);
 unsigned long rlong(void);
 void write_mac_part(unsigned long partnum);
 
-/*
- * Helper functions for command: dump
+/* Helper functions for command: dump
  */
+
 void cmd_helper_dump(void);
 void print_mac_from_nvm(unsigned long partnum);
 void hexdump(unsigned long partnum);
 
-/*
- * Helper functions for command: swap
+/* Helper functions for command: swap
  */
+
 void cmd_helper_swap(void);
 
-/*
- * Helper functions for command: copy
+/* Helper functions for command: copy
  */
+
 void cmd_helper_copy(void);
 
-/*
- * Helper functions for commands:
+/* Helper functions for commands:
  * cat, cat16 and cat128
  */
+
 void cmd_helper_cat(void);
 void cmd_helper_cat16(void);
 void cmd_helper_cat128(void);
 void cat(unsigned long nff);
 void cat_buf(unsigned char *b);
 
+/* Command verification/control
+ */
+
 void check_cmd(void (*fn)(void), const char *name);
 void cmd_helper_err(void);
 
-/*
- * After command processing, write
- * the modified GbE file back.
- *
- * These are stub functions: check
- * below for the actual functions.
+/* Write GbE files to disk
  */
+
 void write_gbe_file(void);
 void set_checksum(unsigned long part);
 unsigned short calculated_checksum(unsigned long p);
 
-/*
- * Helper functions for accessing
- * the NVM area during operation.
+/* NVM read/write
  */
+
 unsigned short nvm_word(unsigned long pos16, unsigned long part);
 void set_nvm_word(unsigned long pos16,
     unsigned long part, unsigned short val16);
@@ -432,23 +384,26 @@ void set_part_modified(unsigned long p);
 void check_nvm_bound(unsigned long pos16, unsigned long part);
 void check_bin(unsigned long a, const char *a_name);
 
-/*
- * Helper functions for stub functions
- * that handle GbE file reads/writes.
+/* GbE file read/write
  */
+
 void rw_gbe_file_part(unsigned long p, int rw_type,
     const char *rw_type_str);
 void write_to_gbe_bin(void);
 int gbe_mv(void);
 void check_written_part(unsigned long p);
 void report_io_err_rw(void);
-int fsync_dir(const char *path);
 unsigned char *gbe_mem_offset(unsigned long part, const char *f_op);
 off_t gbe_file_offset(unsigned long part, const char *f_op);
 off_t gbe_x_offset(unsigned long part, const char *f_op,
     const char *d_type, off_t nsize, off_t ncmp);
 long rw_gbe_file_exact(int fd, unsigned char *mem, unsigned long nrw,
     off_t off, int rw_type);
+
+/* Generic read/write
+ */
+
+int fsync_dir(const char *path);
 long rw_file_exact(int fd, unsigned char *mem, unsigned long len,
     off_t off, int rw_type, int loop_eagain, int loop_eintr,
     unsigned long max_retries, int off_reset);
@@ -466,27 +421,21 @@ off_t lseek_loop(int fd, off_t off,
 #endif
 int try_err(int loop_err, int errval);
 
-/*
- * Error handling and cleanup
+/* Error handling and cleanup
  */
+
 void usage(void);
 void err(int nvm_errval, const char *msg, ...);
 int exit_cleanup(void);
 const char *getnvmprogname(void);
 
-/*
- * a special kind of hell
+/* Portable libc functions
  */
+
 char *new_tmpfile(int *fd, int local, const char *path);
 int x_i_mkstemp(char *template);
 char *x_c_strrchr(const char *s, int c);
-/* x_i_rename not suitable
- * for atomic writes. kept
- * commentted for use in a
- * library in the future */
-/*
 int x_i_rename(const char *src, const char *dst);
-*/
 char *x_c_tmpdir(void);
 int x_i_close(int fd);
 void *x_v_memcpy(void *dst,
@@ -495,9 +444,6 @@ int x_i_memcmp(const void *a,
     const void *b, unsigned long n);
 int x_i_fsync(int fd);
 
-
-
-
 /* asserts */
 
 /* type asserts */

util/nvmutil/lib/checksum.c

@@ -7,31 +7,22 @@
  * Related file: word.c
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
-#include <fcntl.h>
 #include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
-#include <time.h>
-#include <unistd.h>
 
 #include "../include/common.h"
 
 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 +30,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

@@ -5,32 +5,26 @@
  * Command handlers for nvmutil
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
-#include <stdarg.h>
-#include <stddef.h>
 #include <stdio.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #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 +35,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 +100,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 +131,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 +179,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 +191,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 +210,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 +227,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 +250,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 +274,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 +306,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 +327,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 +364,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 +410,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 +437,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 +453,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 +464,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 +475,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 +486,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 +531,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

@@ -5,40 +5,51 @@
  * Safe file handling.
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
-#include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
 #include <fcntl.h>
-#include <limits.h>
-#include <stdarg.h>
-#include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #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 +66,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 +176,7 @@ err_fsync_dir:
 	return -1;
 }
 
-/*
- * create new tmpfile path
+/* create new tmpfile path
  *
  * ON SUCCESS:
  *
@@ -189,14 +199,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 +260,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 +279,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 +397,6 @@ x_c_tmpdir(void)
 	char *t;
 	struct stat st;
 
-	t = getenv("TMPDIR");
 	t = getenv("TMPDIR");
 
 	if (t && *t) {
@@ -409,9 +416,9 @@ x_c_tmpdir(void)
 	return ".";
 }
 
-/*
- * portable mkstemp
+/* portable mkstemp
  */
+
 int
 x_i_mkstemp(char *template)
 {
@@ -436,7 +443,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 +475,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 +491,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 +558,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 +817,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 +838,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 +848,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 +897,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 +960,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,25 +3,18 @@
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
  *
  * I/O functions specific to nvmutil.
- *
- * Related: file.c
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #include "../include/common.h"
@@ -29,30 +22,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 +53,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 +68,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 +87,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 +102,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 +112,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 +123,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 +139,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 +176,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 +187,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 +211,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 +272,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 +282,6 @@ write_to_gbe_bin(void)
 		}
 	}
 
-	/*
-	 * finally:
-	 * must sync to disk!
-	 * very nearly done
-	 */
-
 	if (!f->io_err_gbe_bin)
 		return;
 
@@ -369,9 +295,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 +306,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 +318,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 +360,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 +416,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 +427,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 +435,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 +490,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 +554,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 +576,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 +591,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 +615,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

@@ -9,17 +9,15 @@
 #include <sys/param.h>
 #endif
 #include <sys/types.h>
-#include <sys/stat.h>
 
 #include <errno.h>
-#include <fcntl.h>
+#if !((defined(__OpenBSD__) && (OpenBSD) >= 201) || \
+    defined(__FreeBSD__) || \
+    defined(__NetBSD__) || defined(__APPLE__))
+#include <fcntl.h> /* if not arc4random: /dev/urandom */
+#endif
 #include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #include "../include/common.h"
@@ -45,9 +43,7 @@ hextonum(char ch_s)
 	return 16; /* invalid character */
 }
 
-/*
- * Portable random
- * number generator
+/* Random numbers
  */
 unsigned long
 rlong(void)

util/nvmutil/lib/state.c

@@ -1,17 +1,9 @@
 /* SPDX-License-Identifier: MIT
- *
  * Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
  *
  * 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__
@@ -28,46 +20,21 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #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 +89,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

@@ -1,25 +1,14 @@
 /* SPDX-License-Identifier: MIT
- *
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
  *
  * String handling.
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
-#include <fcntl.h>
-#include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
 #include <unistd.h>
 
 #include "../include/common.h"
@@ -93,67 +82,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/usage.c

@@ -1,35 +1,17 @@
 /* SPDX-License-Identifier: MIT
- *
  * Copyright (c) 2023 Riku Viitanen <riku.viitanen@protonmail.com>
  * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
- *
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
-#include <sys/types.h>
-#include <sys/stat.h>
-
 #include <errno.h>
-#include <fcntl.h>
-#include <limits.h>
-#include <stdarg.h>
-#include <stddef.h>
 #include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
-#include <unistd.h>
 
 #include "../include/common.h"
 
 void
 usage(void)
 {
-	const char *util;
-
-	util = getnvmprogname();
+	const char *util = getnvmprogname();
 
 	fprintf(stderr,
 	    "Modify Intel GbE NVM images e.g. set MAC\n"

util/nvmutil/lib/word.c

@@ -6,22 +6,10 @@
  * words on Intel GbE NVM configurations.
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
-#include <sys/stat.h>
 
 #include <errno.h>
-#include <fcntl.h>
-#include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
-#include <unistd.h>
 
 #include "../include/common.h"
 
@@ -29,20 +17,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 +38,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 +55,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 +65,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");

util/nvmutil/nvmutil.c

@@ -5,142 +5,35 @@
  * 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).
  */
 
-#ifdef __OpenBSD__
-#include <sys/param.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
-#include <time.h>
-#include <unistd.h>
 
 #include "include/common.h"
 
 int
 main(int argc, char *argv[])
 {
-	struct xstate *x;
-	struct commands *cmd;
-	struct xfile *f;
-	unsigned long i;
+	struct xstate *x = xstatus(argc, argv);
+	struct commands *cmd = &x->cmd[x->i];
+	struct xfile *f = &x->f;
 
-	char *tmp_path;
+	unsigned long c;
 
-	struct stat st;
-	int fd;
+	if (cmd->run == NULL)
+		err(errno, "Command not set");
 
-	tmp_path = NULL;
+	cmd->run();
 
-#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 (argc < 3)
-		usage();
-
-#ifdef NVMUTIL_UNVEIL
-	/*
-	 * if global tmp is a different filesystem,
-	 * unveil would trap on final file rename
-	 * and we can't know the path in advance
-	 */
-	tmp_path = new_tmpfile(&fd, 1, NULL);
-#else
-	tmp_path = new_tmpfile(&fd, 0, NULL);
-#endif
-
-	if (tmp_path == NULL)
-		err(errno, "Can't create tmpfile");
-
-#ifdef NVMUTIL_PLEDGE
-#ifdef NVMUTIL_UNVEIL
-	if (pledge("stdio flock rpath wpath cpath unveil", NULL) == -1)
-		err(errno, "pledge, unveil");
-	if (unveil("/dev/null", "r") == -1)
-		err(errno, "unveil: /dev/null");
-#else
-	if (pledge("stdio flock rpath wpath cpath", NULL) == -1)
-		err(errno, "pledge");
-#endif
-#endif
-
-	x = xstatus();
-
-	if (x == NULL)
-		err(errno, NULL);
-	if (x->f.buf == NULL)
-		err(EINVAL, "Work buffer not initialised");
-
-	x->argv0 = argv[0];
-	f = &x->f;
-
-	f->fname = argv[1];
-	f->tname = tmp_path;
-	f->tmp_fd = fd;
-
-	if(fstat(fd, &st) < 0)
-		err(errno, "can't stat tmpfile");
-
-	f->tmp_dev = st.st_dev;
-	f->tmp_ino = st.st_ino;
-
-	sanitize_command_list();
-
-	set_cmd(argc, argv);
-	set_cmd_args(argc, argv);
-
-	i = x->i;
-	cmd = &x->cmd[i];
-
-#ifdef NVMUTIL_UNVEIL
-	if ((cmd->flags & O_ACCMODE) == O_RDONLY) {
-		if (unveil(f->fname, "r") == -1)
-			err(errno, "%s: unveil r", f->fname);
-	} else {
-		if (unveil(f->tname, "rwc") == -1)
-			err(errno, "%s: unveil rw", f->tname);
-	}
-
-	if (unveil(f->tname, "rwc") == -1)
-		err(errno, "%s: unveil rwc", 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(f->buf, 0, GBE_BUF_SIZE);
-	memset(f->bufcmp, 0, GBE_BUF_SIZE);
-
-	copy_gbe();
-
-	read_checksums();
-
-	run_cmd();
+	for (c = 0; c < items(x->cmd); c++)
+		x->cmd[c].run = cmd_helper_err;
 
 	if ((cmd->flags & O_ACCMODE) == O_RDWR)
 		write_to_gbe_bin();

util/nvmutil/todo.c

@@ -1,134 +0,0 @@
-/* SPDX-License-Identifier: MIT
- *
- * Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
- *
- * Five Year Plan
- */
-
-/*
- * Major TODO: split this into multiple files.
- * This program has become quite large now, mostly
- * due to all the extra sanity checks / portability.
- * Make most of nvmutil a *library* for re-use
- *
- * TODO: gettimeofday not posible - use portable functions.
- * TODO: ux fallback: modify the program instead
- * to run on 16-bit systems: smaller buffers, and do
- * operations byte-based instead of word-based.
- *
- * TODO: _XOPEN_SOURCE 500 probably not needed anymore.
- * the portable fallbacks alone are likely enough.
- * e.g. i don't need stdint, and i don't use pwrite/pread
- * anymore.
- *
- * TODO: version detection of various BSDs to detect
- * arc4random, use that if available. but also work on
- * older versions of those BSDs (also MacOS) that lack it.
- *
- * TODO: portability/testing on non-Unix systems:
- * old DOS. all windows versions (probably irrelevant
- * because you can use cygwin/wsl, whatever), classic MacOS,
- * also test really old unix e.g. sunos and irix. Be/Haiku too!
- *
- * TODO: reliance on global variables for status. make
- * functions use structs passed as args instead, make
- * functions re-useable (including libraries), etc.
- *
- * TODO: bound checks for files per-command, e.g. only
- * first 6 bytes for CMD_SETMAC
- *
- * TODO: in command sanitizer: verify that each given
- * entry corresponds to the correct function, in the
- * pointer (this check is currently missing)
- *
- * TODO: general modularisierung of the entire codebase.
- * TODO: better explain copy/swap read inversion trick
- *       by improving existing comments
- * TODO: lots of overwritten comments in code. tidy it up.
- *
- * TODO: use getopt for nvmutil args, so that multiple
- *      operations can be performed, and also on many
- *	files at once (noting limitations with cat)
- *	BONUS: implement own getopt(), for portability
- *
- * TODO: document fuzzing / analysis methods
- * 	for the code, and:
- * TODO: implement rigorous unit tests (separate util)
- *	NOTE: this would *include* known good test files
- *	in various configurations, also invalid files.
- *	the tests would likely be portable posix shell
- *	scripts rather than a new C program, but a modularisiert
- *	codebase would allow me to write a separate C
- *	program to test some finer intricacies
- * TODO: the unit tests would basically test regressions
- * TODO: after writing back a gbe to file, x_i_close() and
- *	open() it again, read it again, and check that
- *	the contents were written correctly, providing
- *	a warning if they were. do this in the main
- *	program.
- * TODO: the unit tests would include an aggressive set
- *	of fuzz tests, under controlled conditions
- *
- * TODO: also document the layout of Intel GbE files, so
- *       that wily individuals can easily expand the
- *	featureset of nvmutil.
- * TODO: write a manpage
- * TODO: simplify the command sanitization, implement more
- *	of it as build time checks, e.g. asserts.
- *	generally remove cleverness from the code, instead
- *	prefyerring readibility
- * TODO: also document nvmutil's coding style, which is
- *	its own style at this point!
- * TODO: when all the above (and possibly more) is done,
- *	submit this tool to coreboot with a further change
- *	to their build system that lets users modify
- *	GbE images, especially set MAC addresses, when
- *	including GbE files in coreboot configs.
- */
-/*
- BONUS TODO:
- CI/CD. woodpecker is good enough, sourcehut also has one.
-	tie this in with other things mentioned here, 
-	e.g. fuzzer / unit tests
-*/
-
-/* Major TODO: reproducible builds
-Test with and without these:
-
-CFLAGS += -fno-record-gcc-switches
-CFLAGS += -ffile-prefix-map=$(PWD)=.
-CFLAGS += -fdebug-prefix-map=$(PWD)=.
-
-I already avoid unique timestamps per-build,
-by not using them, e.g. not reporting build
-time in the program.
-
-When splitting the nvmutil.c file later, do e.g.:
-
-SRC = main.c io.c nvm.c cmd.c
-OBJ = $(SRC:.c=.o)
-
-^ explicitly declare the order in which to build
-*/
-
-/*
-TODO:
-further note when fuzzing is implemented:
-use deterministic randomisation, with a
-guaranteed seed - so e.g. don't use /dev/urandom
-in test builds. e.g. just use normal rand()
-but with a seed e.g. 1234
-*/
-/*
-TODO: stricter build flags, e.g.
-CFLAGS += -fstack-protector-strong
-CFLAGS += -fno-common
-CFLAGS += -D_FORTIFY_SOURCE=2
-CFLAGS += -fPIE
-
-also consider:
--fstack-clash-protection
--Wl,-z,relro
--Wl,-z,now
-*/
-