tidy some comments

Signed-off-by: Leah Rowe <leah@libreboot.org>
This commit is contained in:
Leah Rowe
2026-03-19 16:02:15 +00:00
parent 7ad924a91f
commit 55f006318a
10 changed files with 109 additions and 207 deletions

View File

@@ -1,6 +1,4 @@
/*
* SPDX-License-Identifier: MIT
*
/* SPDX-License-Identifier: MIT
* Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
*/
@@ -422,7 +420,7 @@ const char *getnvmprogname(void);
char *new_tmpfile(int *fd, int local, const char *path);
int mkstemp_n(char *template);
char *x_c_tmpdir(void);
char *get_tmpdir(void);
int close_on_eintr(int fd);
int fsync_on_eintr(int fd);
@@ -487,6 +485,5 @@ typedef char bool_no_loop_eagain[(NO_LOOP_EAGAIN==0)?1:-1];
typedef char bool_off_err[(OFF_ERR==0)?1:-1];
typedef char bool_off_reset[(OFF_RESET==0||OFF_RESET==1)?1:-1];
#endif
#endif

View File

@@ -1,10 +1,7 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
*
* Functions related to GbE NVM checksums.
*
* Related file: word.c
*/
#include <sys/types.h>
@@ -42,15 +39,14 @@ read_checksums(void)
if (cmd->arg_part)
_max_invalid = 1;
/*
* Skip verification on this part,
/* Skip verification on this part,
* but only when arg_part is set.
*/
_skip_part = f->part ^ 1;
for (_p = 0; _p < 2; _p++) {
/*
* Only verify a part if it was *read*
/* Only verify a part if it was *read*
*/
if (cmd->arg_part && (_p == _skip_part))
continue;
@@ -61,9 +57,11 @@ read_checksums(void)
}
if (_num_invalid >= _max_invalid) {
if (_max_invalid == 1)
err(ECANCELED, "%s: part %lu has a bad checksum",
f->fname, (unsigned long)f->part);
err(ECANCELED, "%s: No valid checksum found in file",
f->fname);
}

View File

@@ -1,8 +1,5 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
*
* Command handlers for nvmutil
*/
#include <sys/types.h>
@@ -35,9 +32,6 @@ sanitize_command_list(void)
sanitize_command_index(c);
}
/* TODO: specific config checks per command
*/
void
sanitize_command_index(unsigned long c)
{
@@ -142,7 +136,8 @@ set_cmd_args(int argc, char *argv[])
if (x->no_cmd)
usage();
/* Maintainer bugs */
/* Maintainer bug
*/
if (cmd->arg_part && argc < 4)
err(EINVAL,
"arg_part set for command that needs argc4");
@@ -172,7 +167,8 @@ conv_argv_part_num(const char *part_str)
if (part_str[0] == '\0' || part_str[1] != '\0')
err(EINVAL, "Partnum string '%s' wrong length", part_str);
/* char signedness is implementation-defined */
/* char signedness is implementation-defined
*/
ch = (unsigned char)part_str[0];
if (ch < '0' || ch > '1')
err(EINVAL, "Bad part number (%c)", ch);
@@ -286,16 +282,14 @@ set_mac_nib(unsigned long mac_str_pos,
err(EINVAL, "Invalid character '%c'",
mac->str[mac_str_pos + mac_nib_pos]);
/*
* If random, ensure that local/unicast bits are set.
/* If random, ensure that local/unicast bits are set.
*/
if ((mac_byte_pos == 0) && (mac_nib_pos == 1) &&
((mac_ch | 0x20) == 'x' ||
(mac_ch == '?')))
hex_num = (hex_num & 0xE) | 2; /* local, unicast */
/*
* MAC words stored big endian in-file, little-endian
/* MAC words stored big endian in-file, little-endian
* logically, so we reverse the order.
*/
mac->mac_buf[mac_byte_pos >> 1] |= hex_num <<

View File

@@ -1,8 +1,5 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
*
* Safe file handling.
*/
#include <sys/types.h>
@@ -51,6 +48,9 @@ err_same_file:
return -1;
}
/* open() but with abort traps
*/
void
xopen(int *fd_ptr, const char *path, int flags, struct stat *st)
{
@@ -67,8 +67,8 @@ 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
* directory containing the target file.
/* fsync() the directory of a file,
* useful for atomic writes
*/
int
@@ -177,28 +177,11 @@ err_fsync_dir:
return -1;
}
/* create new tmpfile path
/* returns ptr to path (string). if local>0:
* make tmpfile in the same directory as the
* file. if local==0, use TMPDIR
*
* ON SUCCESS:
*
* returns ptr to path string on success
* ALSO: the int at *fd will be set,
* indicating the file descriptor
*
* ON ERROR:
*
* return NULL (*fd not touched)
*
* malloc() may set errno, but you should
* not rely on errno from this function
*
* local: if non-zero, then only a file
* name will be given, relative to
* the current file name. for this,
* the 3rd argument (path) must be non-null
*
* if local is zero, then 3rd arg (path)
* is irrelevant and can be NULL
* if local==0, the 3rd argument is ignored
*/
char *
@@ -225,16 +208,6 @@ new_tmpfile(int *fd, int local, const char *path)
int fd_tmp = -1;
int flags;
/*
* 256 is the most
* conservative path
* size limit (posix),
* but 4096 is modern
*
* set PATH_LEN as you
* wish, at build time
*/
#if defined(PATH_LEN) && \
(PATH_LEN) >= 256
maxlen = PATH_LEN;
@@ -265,7 +238,7 @@ new_tmpfile(int *fd, int local, const char *path)
*/
tmpdir_len = xstrxlen(default_tmpname, maxlen);
} else {
base = x_c_tmpdir();
base = get_tmpdir();
if (base == NULL)
base = tmp_default;
@@ -392,8 +365,12 @@ lock_file(int fd, int flags)
return 0;
}
/* return TMPDIR, or fall back
* to portable defaults
*/
char *
x_c_tmpdir(void)
get_tmpdir(void)
{
char *t;
struct stat st;
@@ -401,9 +378,12 @@ x_c_tmpdir(void)
t = getenv("TMPDIR");
if (t && *t) {
if (stat(t, &st) == 0 && S_ISDIR(st.st_mode)) {
if ((st.st_mode & S_IWOTH) && !(st.st_mode & S_ISVTX))
return NULL;
return t;
}
}
@@ -434,11 +414,11 @@ mkstemp_n(char *template)
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
unsigned long r;
unsigned long max_len =
#ifndef PATH_LEN
4096;
#if defined(PATH_LEN) && \
(PATH_LEN) >= 256
unsigned long max_len = PATH_LEN;
#else
(PATH_LEN);
unsigned long max_len = 4096;
#endif
len = xstrxlen(template, max_len);
@@ -579,45 +559,32 @@ err_rw_file_exact:
return -1;
}
/* prw() - portable read-write
/* prw() - portable read-write with more
* safety checks than barebones libc
*
* This implements a portable analog of pwrite()
* and pread() - note that this version is not
* thread-safe (race conditions are possible on
* shared file descriptors).
*
* This limitation is acceptable, since nvmutil is
* single-threaded. Portability is the main goal.
* portable pwrite/pread on request, or real
* pwrite/pread libc functions can be used.
* the portable (non-libc) pread/pwrite is not
* thread-safe, because it does not prevent or
* mitigate race conditions on file descriptors
*
* If you need real pwrite/pread, just compile
* with flag: HAVE_REAL_PREAD_PWRITE=1
*
* A fallback is provided for regular read/write.
* rw_type can be IO_READ, IO_WRITE, IO_PREAD
* or IO_PWRITE
* rw_type can be IO_READ (read), IO_WRITE (write),
* IO_PREAD (pread) or IO_PWRITE
*
* loop_eagain does a retry loop on EAGAIN if set
* loop_eintr does a retry loop on EINTR if set
*
* Unlike the bare syscalls, prw() does security
* checks e.g. checks NULL strings, checks bounds,
* also mitigates a few theoretical libc bugs.
* It is designed for extremely safe single-threaded
* I/O on applications that need it.
* race conditions on non-libc pread/pwrite:
* if a file offset changes, abort, to mitage.
*
* NOTE: If you use loop_eagain (1), you enable wait
* loop on EAGAIN. Beware if using this on a non-blocking
* pipe (it could spin indefinitely).
*
* off_reset: if zero, and using fallback pwrite/pread
* analogs, we check if a file offset changed,
* which would indicate another thread changed
* it, and return error, without resetting the
* file - this would allow that thread to keep
* running, but we could then cause a whole
* program exit if we wanted to.
* if not zero:
* we reset and continue, and pray for the worst.
* off_reset 1: reset the file offset *once* if
* a change was detected, assuming
* nothing else is touching it now
* off_reset 0: never reset if changed
*/
long
@@ -644,16 +611,15 @@ prw(int fd, void *mem, unsigned long nrw,
r = -1;
/* Programs like cat can use this,
so we only check if it's a normal
file if not looping EAGAIN */
/* do not use loop_eagain on
* normal files
*/
if (!loop_eagain) {
/*
* Checking on every run of prw()
* is expensive if called many
* times, but is defensive in
* case the status changes.
/* check whether the file
* changed
*/
if (check_file(fd, &st) == -1)
return -1;
}
@@ -703,39 +669,21 @@ real_pread_pwrite:
verified = lseek_on_eintr(fd, (off_t)0, SEEK_CUR,
loop_eagain, loop_eintr);
/*
* Partial thread-safety: detect
* if the offset changed to what
* we previously got. If it did,
* then another thread may have
* changed it. Enabled if
* off_reset is OFF_RESET.
*
* We do this *once*, on the theory
* that nothing is touching it now.
/* abort if the offset changed,
* indicating race condition. if
* off_reset enabled, reset *ONCE*
*/
if (off_reset && off != verified)
lseek_on_eintr(fd, off, SEEK_SET,
loop_eagain, loop_eintr);
do {
/*
* Verify again before I/O
* (even with OFF_ERR)
*
* This implements the first check
* even with OFF_ERR, but without
* the recovery. On ERR_RESET, if
* the check fails again, then we
* know something else is touching
* the file, so it's best that we
* probably leave it alone and err.
*
* In other words, ERR_RESET only
* tolerates one change. Any more
* will cause an exit, including
* per EINTR/EAGAIN re-spin.
/* check offset again, repeatedly.
* even if off_reset is set, this
* aborts if offsets change again
*/
verified = lseek_on_eintr(fd, (off_t)0, SEEK_CUR,
loop_eagain, loop_eintr);
@@ -831,9 +779,7 @@ err_is_file:
return -1;
}
/* Check weirdness on buggy libc.
*
* POSIX can say whatever it wants.
/* POSIX can say whatever it wants.
* specification != implementation
*/
@@ -888,11 +834,6 @@ err_rw_over_nrw:
#if !defined(HAVE_REAL_PREAD_PWRITE) || \
HAVE_REAL_PREAD_PWRITE < 1
/*
* lseek_on_eintr() does lseek() but optionally
* on an EINTR/EAGAIN wait loop. Used by prw()
* for setting offsets for positional I/O.
*/
off_t
lseek_on_eintr(int fd, off_t off, int whence,
int loop_eagain, int loop_eintr)

View File

@@ -1,5 +1,4 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
*
* I/O functions specific to nvmutil.
@@ -44,12 +43,11 @@ open_gbe_file(void)
if (_flags == -1)
err(errno, "%s: fcntl(F_GETFL)", f->fname);
/*
* O_APPEND must not be used, because this
* allows POSIX write() to ignore the
* current write offset and write at EOF,
* which would therefore break pread/pwrite
/* O_APPEND allows POSIX write() to ignore
* the current write offset and write at EOF,
* which would break positional read/write
*/
if (_flags & O_APPEND)
err(EIO, "%s: O_APPEND flag", f->fname);
@@ -223,10 +221,10 @@ write_to_gbe_bin(void)
write_gbe_file();
/*
* We may otherwise read from
/* We may otherwise read from
* cache, so we must sync.
*/
if (fsync_on_eintr(f->tmp_fd) == -1)
err(errno, "%s: fsync (pre-verification)",
f->tname);
@@ -239,11 +237,6 @@ write_to_gbe_bin(void)
if (f->io_err_gbe)
err(EIO, "%s: bad write", f->fname);
/*
* success!
* now just rename the tmpfile
*/
saved_errno = errno;
if (close_on_eintr(f->tmp_fd) == -1) {
@@ -258,6 +251,11 @@ write_to_gbe_bin(void)
errno = saved_errno;
/* tmpfile written, now we
* rename it back to the main file
* (we do atomic writes)
*/
f->tmp_fd = -1;
f->gbe_fd = -1;
@@ -275,6 +273,7 @@ write_to_gbe_bin(void)
/* removed by rename
*/
if (f->tname != NULL)
free(f->tname);
@@ -338,17 +337,17 @@ check_written_part(unsigned long p)
f->rw_check_partial_read[p])
return;
/*
* We only load one part on-file, into memory but
/* We only load one part on-file, into memory but
* always at offset zero, for post-write checks.
* That's why we hardcode good_checksum(0).
* That's why we hardcode good_checksum(0)
*/
buf_restore = f->buf;
/*
* good_checksum works on f->buf
/* good_checksum works on f->buf
* so let's change f->buf for now
*/
f->buf = f->pad;
if (good_checksum(0))
@@ -427,7 +426,8 @@ gbe_mv(void)
char *dest_tmp;
int dest_fd;
/* will be set 0 if it doesn't */
/* will be set 0 if it doesn't
*/
tmp_gbe_bin_exists = 1;
dest_tmp = NULL;
@@ -438,8 +438,8 @@ gbe_mv(void)
rval = rename(f->tname, f->fname);
if (rval > -1) {
/*
* same filesystem
/* rename on same filesystem
*/
tmp_gbe_bin_exists = 0;
@@ -455,19 +455,22 @@ gbe_mv(void)
if (errno != EXDEV)
goto ret_gbe_mv;
/* cross-filesystem rename */
/*
* OR, cross-filesystem rename:
*/
if ((rval = f->tmp_fd = open(f->tname,
O_RDONLY | O_BINARY)) == -1)
goto ret_gbe_mv;
/* create replacement temp in target directory */
/* create replacement temp in target directory
*/
dest_tmp = new_tmpfile(&dest_fd, 1, f->fname);
if (dest_tmp == NULL)
goto ret_gbe_mv;
/* copy data */
/* copy data
*/
rval = rw_file_exact(f->tmp_fd, f->bufcmp,
f->gbe_file_size, 0, IO_PREAD,
NO_LOOP_EAGAIN, LOOP_EINTR,
@@ -520,8 +523,7 @@ ret_gbe_mv:
f->tmp_fd = -1;
}
/*
* before this function is called,
/* before this function is called,
* tmp_fd may have been moved
*/
if (tmp_gbe_bin_exists) {
@@ -532,8 +534,7 @@ ret_gbe_mv:
}
if (rval < 0) {
/*
* if nothing set errno,
/* if nothing set errno,
* we assume EIO, or we
* use what was set
*/
@@ -546,8 +547,7 @@ ret_gbe_mv:
return rval;
}
/*
* This one is similar to gbe_file_offset,
/* This one is similar to gbe_file_offset,
* but used to check Gbe bounds in memory,
* and it is *also* used during file I/O.
*/
@@ -566,12 +566,9 @@ gbe_mem_offset(unsigned long p, const char *f_op)
(f->buf + (unsigned long)gbe_off);
}
/*
* I/O operations filtered here. These operations must
/* I/O operations filtered here. These operations must
* only write from the 0th position or the half position
* within the GbE file, and write 4KB of data.
*
* This check is called, to ensure just that.
*/
off_t
gbe_file_offset(unsigned long p, const char *f_op)

View File

@@ -1,5 +1,4 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
*
* Numerical functions.
@@ -45,6 +44,7 @@ hextonum(char ch_s)
/* Random numbers
*/
unsigned long
rlong(void)
{

View File

@@ -1,9 +1,7 @@
/* 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.
* State machine (singleton) for nvmutil data.
*/
#ifdef __OpenBSD__
@@ -24,12 +22,6 @@
#include "../include/common.h"
/*
* Initialise program state,
* load GbE file and verify
* data, ready for operation
* (singleton design)
*/
struct xstate *
xstatus(int argc, char *argv[])
{

View File

@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: MIT
* Copyright (c) 2026 Leah Rowe <leah@libreboot.org>
*
* String handling.
* String functions
*/
#include <sys/types.h>
@@ -13,10 +13,10 @@
#include "../include/common.h"
/*
* Portable strcmp() but blocks NULL/empty/unterminated
* strings. Even stricter than strncmp().
/* Portable strncmp() that blocks
* NULL/empty/unterminated strings
*/
int
xstrxcmp(const char *a, const char *b, unsigned long maxlen)
{
@@ -43,24 +43,16 @@ xstrxcmp(const char *a, const char *b, unsigned long maxlen)
return ac - bc;
}
/*
* We reached maxlen, so assume unterminated string.
*/
err(EINVAL, "Unterminated string in xstrxcmp");
/*
* Should never reach here. This keeps compilers happy.
*/
errno = EINVAL;
return -1;
}
/*
* strnlen() but aborts on NULL input, and empty strings.
* Our version also prohibits unterminated strings.
* strnlen() was standardized in POSIX.1-2008 and is not
* available on some older systems, so we provide our own.
/* Portable strncmp() that blocks
* NULL/empty/unterminated strings
*/
unsigned long
xstrxlen(const char *scmp, unsigned long maxlen)
{

View File

@@ -1,9 +1,8 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
*
* Manipulate sixteen-bit little-endian
* words on Intel GbE NVM configurations.
* Manipulate Intel GbE NVM words, which are 16-bit little
* endian in the files (MAC address words are big endian).
*/
#include <sys/types.h>
@@ -13,13 +12,6 @@
#include "../include/common.h"
/*
* 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-file.
*/
unsigned short
nvm_word(unsigned long pos16, unsigned long p)
{

View File

@@ -1,5 +1,4 @@
/* SPDX-License-Identifier: MIT
*
* Copyright (c) 2022-2026 Leah Rowe <leah@libreboot.org>
*
* This tool lets you modify Intel GbE NVM (Gigabit Ethernet