libdevs: isolate platform-dependent code, and port f3probe to macOS by sbrunner-atx · Pull Request #293 · AltraMayor/f3

sbrunner-atx · 2026-10-18T21:54:30Z

Follow-up to #283, following the structure @AltraMayor proposed there. #283 stays open; this PR carries only the restructure plus f3probe, with f3brew and f3fix to come separately.

The restructure

libdevs.c reached for Linux-only interfaces (O_DIRECT, libudev, BLKGETSIZE64/BLKSSZGET, USBDEVFS_RESET) from several places in the block-device backend. It now keeps the portable logic and includes exactly one platform file:

#if defined(__linux__)
#include "linux_libdevs.c"
#elif defined(__APPLE__)
#include "darwin_libdevs.c"
#else
#error "Platform not supported"
#endif

Each platform file defines six static functions — bdev_open(), bdev_drop_cache(), bdev_validate_disk(), bdev_geometry(), bdev_manual_usb_reset(), bdev_usb_reset() — and the contract is documented above the include. libdevs.c is left with two preprocessor blocks: the dispatch above, and a _DARWIN_C_SOURCE feature-test macro at the top, which mirrors what libfile.c already does upstream (_POSIX_C_SOURCE otherwise hides F_NOCACHE and the DKIOC* ioctls). No platform logic remains in the file; in #283 it was spread across eleven sites.

linux_libdevs.c is the existing code moved, not rewritten. The only new Linux code is the thin wrappers: bdev_validate_disk() is the udev validation lifted out of create_block_device() (it now cleans up its own handles and returns 0/-1 instead of jumping to labels in the caller), and bdev_geometry() wraps the two BLK* ioctls.

macOS

darwin_libdevs.c implements the same contract: F_NOCACHE instead of O_DIRECT, DKIOCGETBLOCKCOUNT/DKIOCGETBLOCKSIZE instead of the BLK* ioctls, and whole-disk detection by device-name convention instead of libudev, so pointing f3probe at a partition is still caught.

On Darwin the Makefile drops -ludev, limits make extra to f3probe, and resolves argp-standalone via brew --prefix argp-standalone — the formula is keg-only, so its headers are not linked into $(brew --prefix) and the build failed out of the box. That fix is @nickclyde's from #285; credit to him.

Testing

Linux (Debian, gcc): make all extra builds f3write, f3read, f3probe, f3brew and f3fix clean under -Wall -Wextra -pedantic -Wdeclaration-after-statement. The Linux code path is unchanged.

macOS 26.5, Intel: make all extra builds clean, no warnings. Against a 20 MB hdiutil RAM disk:

Good news: The device `/dev/rdisk1' is the real thing

Device geometry:
	         *Usable* size: 20.00 MB (40960 blocks)
	        Announced size: 20.00 MB (40960 blocks)
	                Module: 32.00 MB (2^25 Bytes)
	Approximate cache size: 0.00 Bytes (0 blocks)
	   Physical block size: 512.00 Bytes (2^9 Bytes)

And a partition is refused rather than probed:

$ ./build/f3probe /dev/rdisk1s1
Device `/dev/rdisk1s1' is a partition of disk device `/dev/rdisk1'.
You must run f3probe on the disk device as follows:
f3probe /dev/rdisk1

Earlier, the same macOS device code in #283 probed a genuine SanDisk Ultra 128 GB correctly (usable size = announced size, 114.60 GB), and that branch has been in use through a Homebrew tap on Intel hardware.

Two things worth a reviewer's attention

bdev_manual_usb_reset() and bdev_usb_reset() have to exist because create_block_device() references them, but f3probe passes RT_NONE and never calls either. The macOS reset paths are therefore unexercised by this PR — they will get their first real test with the f3brew port. Happy to replace both with -EOPNOTSUPP stubs here if you would rather not carry untested code.

bdev_validate_disk() on macOS does not check that the disk is backed by USB; there is no libudev equivalent to hand. With RT_NONE this does not matter, but it will need an answer for f3brew, and I would welcome suggestions.

@mgarciaisaia @nickclyde — review very welcome, particularly on Apple Silicon, which I have no way to test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EL8ecmd96pRXMPUrYMk416

The block-device backend in libdevs.c reached for Linux-only interfaces (O_DIRECT, libudev, BLKGETSIZE64/BLKSSZGET and USBDEVFS_RESET) from several places, so adding a second platform meant scattering conditionals through the file. Move that code behind a small platform layer. libdevs.c keeps the portable logic and includes exactly one platform file: #if defined(__linux__) #include "linux_libdevs.c" #elif defined(__APPLE__) #include "darwin_libdevs.c" #else #error "Platform not supported" #endif Each platform file defines bdev_open(), bdev_drop_cache(), bdev_validate_disk(), bdev_geometry(), bdev_manual_usb_reset() and bdev_usb_reset(); the contract is documented above the include. linux_libdevs.c is the existing code moved, not rewritten, so the Linux build keeps its current behaviour. darwin_libdevs.c implements the same contract on macOS: F_NOCACHE in place of O_DIRECT, DKIOCGETBLOCKCOUNT/DKIOCGETBLOCKSIZE in place of the BLK* ioctls, and whole-disk detection by device-name convention in place of libudev, so running f3probe on a partition is still caught. Only f3probe is built on macOS for now; f3brew and f3fix follow in their own patches. On Darwin the Makefile drops -ludev and resolves argp-standalone through `brew --prefix argp-standalone`, since the formula is keg-only and its headers are not linked into the prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EL8ecmd96pRXMPUrYMk416

AltraMayor

libdevs: isolate platform-dependent code, and port f3probe to macOS

AltraMayor · 2026-10-26T12:50:38Z

 	ARGP = /usr/local
 	ifeq ($(OS), Darwin)
 		ifneq ($(shell command -v brew),)
-			ARGP = $(shell brew --prefix)
+			# argp-standalone is keg-only, so its headers are not
+			# linked into $(shell brew --prefix).
+			ARGP = $(shell brew --prefix argp-standalone 2>/dev/null \
+				|| brew --prefix)
 		endif
 	endif
 	CFLAGS += -I$(ARGP)/include
 	LDFLAGS += -L$(ARGP)/lib -largp
 endif

+ifeq ($(OS), Darwin)
+	LDLIBS_UDEV =
+	# Only f3probe is ported to macOS so far; f3brew and f3fix
+	# follow in their own patches.
+	EXTRA_TARGETS = $(BUILD_DIR)/f3probe
+else
+	LDLIBS_UDEV = -ludev
+endif


Instead of having two ifeq ($(OS), Darwin) blocks, and since libudev is Linux-specific, we can combine these into a single ifeq ($(OS), Linux) ... else ... endif block:

ifeq ($(OS), Linux) LDLIBS_UDEV = -ludev else ARGP = /usr/local ifeq ($(OS), Darwin) # Only f3probe is ported to macOS so far; f3brew and f3fix # follow in their own patches. EXTRA_TARGETS = $(BUILD_DIR)/f3probe ifneq ($(shell command -v brew),) # argp-standalone is keg-only, so its headers are not # linked into $(shell brew --prefix). ARGP = $(shell brew --prefix argp-standalone 2>/dev/null \ || brew --prefix) endif endif CFLAGS += -I$(ARGP)/include LDFLAGS += -L$(ARGP)/lib -largp endif

AltraMayor · 2026-10-26T12:50:41Z

 $(BUILD_DIR)/f3fix: $(BUILD_DIR)/libutils.o $(BUILD_DIR)/f3fix.o
 	$(CC) -o $@ $^ $(LDFLAGS) -lparted

+$(BUILD_DIR)/libdevs.o: $(SRC_DIR)/linux_libdevs.c $(SRC_DIR)/darwin_libdevs.c


This explicit dependency rule is not needed: CFLAGS already includes -MMD and -include $(BUILD_DIR)/*.d below automatically tracks whichever *_libdevs.c file was #included on the host platform.

AltraMayor · 2026-10-26T12:50:44Z

+#if __APPLE__ && __MACH__
+/* _POSIX_C_SOURCE hides F_NOCACHE and the DKIOC* ioctls. */
+#define _DARWIN_C_SOURCE
+#endif	/* Apple Macintosh */


Let's use #if defined(__APPLE__) && defined(__MACH__) (or #elif defined(__APPLE__)) consistently here and at line 476 below.

AltraMayor · 2026-10-26T12:50:46Z

+ *   int bdev_drop_cache(int fd);
+ *	Drop any cached pages for @fd after a write.  Return 0 on
+ *	success, or a negative error code.


posix_fadvise(2) in linux_libdevs.c returns a positive error number on failure rather than a negative error code (and bdev_write_blocks() and its callers only test for a non-zero return value). Please update the comment to say "or a non-zero error code" (or negate the return of posix_fadvise(2)).

Also, please remove the extra double blank lines added around this block and between functions below (lines 437, 481, 496, 518, and 538) so top-level definitions remain separated by a single blank line.

AltraMayor · 2026-10-26T12:50:50Z

+		udev_device_unref(usb_dev);
+	}
+
+	udev_device_unref(fd_dev);
+	assert(!udev_unref(udev));
+	return 0;
+
+fd_dev:
+	udev_device_unref(fd_dev);
+udev:
+	assert(!udev_unref(udev));


We can avoid duplicating udev_device_unref(fd_dev); and assert(!udev_unref(udev)); by using int rc = -1;, setting rc = 0; on the success path, and falling through to fd_dev: (returning rc at the end), matching the goto cleanup pattern used in the functions above.

AltraMayor · 2026-10-26T12:50:54Z

+static inline int bdev_open(const char *filename)
+{
+	/* macOS has no O_DIRECT; F_NOCACHE turns the page cache off for
+	 * this descriptor, which is what the probe algorithm requires.
+	 * Callers should prefer the raw device (/dev/rdiskN); see
+	 * bdev_validate_disk().
+	 */
+	int fd = open(filename, O_RDWR);
+	if (fd >= 0 && fcntl(fd, F_NOCACHE, 1))
+		warn("Can't disable caching for device `%s'", filename);
+	return fd;
+}


As the comment states, disabling the page cache is required by the probe algorithm. If fcntl(fd, F_NOCACHE, 1) fails, merely warning and returning fd will allow f3probe to run against the page cache and produce bogus results. bdev_open() should fail when F_NOCACHE cannot be enabled:

int fd = open(filename, O_RDWR); if (fd >= 0 && fcntl(fd, F_NOCACHE, 1)) { int saved_errno = errno; assert(!close(fd)); errno = saved_errno; return -1; } return fd;

AltraMayor · 2026-10-26T12:50:56Z

+static bool darwin_is_whole_disk(const char *filename, char *disk_name,
+	size_t disk_name_len)
+{
+	const char *p = filename;
+	const char *num_start;
+
+	if (strncmp(p, "/dev/", 5))
+		return false;
+	p += 5;
+	if (*p == 'r')
+		p++;
+	if (strncmp(p, "disk", 4))
+		return false;
+	p += 4;
+
+	num_start = p;
+	while (*p >= '0' && *p <= '9')
+		p++;
+	if (p == num_start)
+		return false;
+	if (*p == '\0')
+		return true;
+
+	if (*p == 's')
+		snprintf(disk_name, disk_name_len, "/dev/rdisk%.*s",
+			(int)(p - num_start), num_start);
+	return false;
+}
+
+static int bdev_validate_disk(const char *filename, int fd,
+	enum reset_type rt)
+{
+	char disk_name[32] = "";
+
+	UNUSED(fd);
+
+	if (!darwin_is_whole_disk(filename, disk_name, sizeof(disk_name))) {
+		if (disk_name[0])
+			fprintf(stderr, "Device `%s' is a partition of disk device `%s'.\n"
+				"You must run %s on the disk device as follows:\n"
+				"%s %s\n",
+				filename, disk_name, __progname,
+				__progname, disk_name);
+		else
+			fprintf(stderr, "Device `%s' is not a whole-disk device.\n"
+				"Expected a name like /dev/rdisk5; see `diskutil list'.\n",
+				filename);
+		return -1;
+	}
+
+	if (strncmp(filename, "/dev/rdisk", 10))
+		fprintf(stderr, "NOTE: prefer the raw device (/dev/r%s) for correct operation.\n",
+			filename + 5);
+
+	/* macOS has no equivalent of libudev's device tree here, so
+	 * whether the disk is backed by USB is not checked; the reset
+	 * implementations below report it instead.
+	 */
+	UNUSED(rt);
+	return 0;
+}


A few points on darwin_is_whole_disk() and bdev_validate_disk():

Require /dev/rdiskN instead of printing a NOTE:: On macOS, /dev/diskN is the buffered block device whereas /dev/rdiskN is the raw character device needed for unbuffered I/O. Rather than printing a NOTE: at lines 109–111 and continuing on /dev/diskN, bdev_validate_disk() should reject /dev/diskN (return -1) and instruct the user to run %s /dev/r%s, just like it does for partitions.

Validate fd with fstat(2): Like dev_from_block_fd() in linux_libdevs.c, please check fstat(fd, &fd_stat) and verify that fd is a character device (S_ISCHR(fd_stat.st_mode)) so that a regular file is not accepted even if its name matches /dev/rdiskN.

Slice suffix check: At line 82, *p == 's' matches even if no digits follow 's' (e.g. /dev/rdisk5s). Please verify that 's' is followed by at least one digit (p[1] >= '0' && p[1] <= '9').

Style: Please add braces { ... } around the multi-line if / else bodies at lines 96–106.

AltraMayor · 2026-10-26T12:50:57Z

+static int bdev_manual_usb_reset(struct device *dev)
+{
+	struct block_device *bdev = dev_bdev(dev);
+	uint64_t orig_size = dev_get_size_byte(dev);
+	int waited_ms;
+
+	if (bdev->fd < 0)
+		return - EBADF;
+	assert(!close(bdev->fd));
+	bdev->fd = -1;
+
+	printf("Please unplug and plug back the USB drive. Waiting...");
+	fflush(stdout);
+
+	/* Wait for the device to go away. */
+	while (1) {
+		struct stat st;
+		if (stat(bdev->filename, &st))
+			break;
+		msleep_ms(200);
+	}
+
+	/* Wait up to 60s for it to come back with the same size. */
+	for (waited_ms = 0; waited_ms < 60000; waited_ms += 200) {
+		int fd = bdev_open(bdev->filename);
+		if (fd >= 0) {
+			uint64_t size_byte;
+			int block_size;
+			if (!bdev_geometry(fd, &size_byte, &block_size) &&
+				size_byte == orig_size) {
+				bdev->fd = fd;
+				printf(" Thanks\n\n");
+				return 0;
+			}
+			assert(!close(fd));
+		}
+		msleep_ms(200);
+	}
+
+	warnx("Device `%s' did not come back after the reset; "
+		"macOS may have renumbered the disk. Check with "
+		"`diskutil list' and run again.", bdev->filename);
+	return - ENODEV;
+}
+
+static int bdev_usb_reset(struct device *dev)
+{
+	UNUSED(dev);
+	warnx("Software USB reset is not supported on macOS; "
+		"use the manual reset (unplug/replug) instead");
+	return - EOPNOTSUPP;
+}


Regarding the question in the PR description: yes, let's replace bdev_manual_usb_reset() with a stub returning -EOPNOTSUPP (and drop msleep_ms() above, and assert/reject rt != RT_NONE in bdev_validate_disk()) so that this PR does not carry untested f3brew code.

When we get to the f3brew PR, a couple of things to keep in mind for macOS resets and USB validation:

Polling bdev_open() after unplug/replug on macOS will typically fail with EBUSY because diskarbitrationd auto-mounts filesystems as soon as the USB drive is plugged back in.

Polling the same /dev/rdiskN filename also breaks if macOS renumbers the disk or if a different drive of the same size is plugged in.

On macOS, IOKit (IOBSDNameMatching and walking up kIOServicePlane for IOUSBHostDevice / IOUSBDevice) and DiskArbitration can both check whether a disk is backed by USB and track/unmount the device across resets by USB serial number.

AltraMayor · 2026-10-26T17:15:28Z

 	return read_all(bdev->fd, buf, length);
 }

+


Please drop this blank line.

AltraMayor · 2026-10-26T17:24:32Z

+/* Platform layer.
+ *
+ * The platform-dependent part of the block-device backend lives in a
+ * per-platform file included below.  Each of those files must define
+ * the following static functions:


This documentation is a nice touch. Thanks.

Please add function prototypes so the compiler can check them for whatever platform is compiling.

AltraMayor · 2026-10-26T17:41:35Z

Please copy the f3probe test cases from Linux to macOS in .github/workflows/test.yml.

AltraMayor requested changes Oct 26, 2026

View reviewed changes

AltraMayor added the Feature request Features that users have expressed interest in. label Oct 26, 2026

Navigation Menu

Repository navigation

libdevs: isolate platform-dependent code, and port f3probe to macOS - #293

libdevs: isolate platform-dependent code, and port f3probe to macOS#293
sbrunner-atx wants to merge 1 commit into
AltraMayor:masterfrom
sbrunner-atx:macos-probe-split

sbrunner-atx commented Oct 18, 2026 •

edited

Loading

Uh oh!

AltraMayor left a comment

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor Oct 26, 2026

Uh oh!

AltraMayor commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sbrunner-atx commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The restructure

macOS

Testing

Two things worth a reviewer's attention

Uh oh!

AltraMayor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AltraMayor commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sbrunner-atx commented Oct 18, 2026 •

edited

Loading