libfrog: wrap handle construction code

Clean up all the open-coded logic to construct a file handle from a
fshandle and some bulkstat/parent pointer information.  The new
functions are stashed in a private header file to avoid leaking the
details of xfs_handle construction in the public libhandle headers.

I tried moving the code to libhandle, but I don't entirely like the
result.  The libhandle functions pass around handles as arbitrary binary
blobs that come from and are sent to the kernel, meaning that the
interface is full of (void *, size_t) tuples.  Putting these new
functions in libhandle breaks that abstraction because now clients know
that they can deal with a struct xfs_handle.

We could fix that leak by changing it to a (void *, size_t) tuple, but
then we'd have to validate the size_t or returns -1 having set errno,
which then means that all the client code now has to have error handling
for a case that we're fairly sure can't be true.  This is overkill for
xfsprogs code that knows better, because we can trust ourselves to know
the exact layout of a handle.

So this nice compact code:

	memcpy(&handle.ha_fsid, file->fshandle, file->fshandle_len);
	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
				sizeof(handle.ha_fid.fid_len);

becomes:

	ret = handle_from_fshandle(&handle, file->fshandle,
			file->fshandle_len);
	if (ret) {
		perror("what?");
		return -1;
	}

Which is much more verbose code, and right now it exists to handle an
exceptional condition that is not possible.  If someone outside of
xfsprogs would like this sort of functionality in libhandle I'm all for
adding it, but with zero demand from external users, I prefer to keep
things simple.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/io/parent.c b/io/parent.c
index 8db93d9875528999802c08b0d919b8d15b754261..3ba3aef48cb9be66e1a19799084ac955a770d29f 100644 (file)
--- a/io/parent.c
+++ b/io/parent.c
@@ -11,6 +11,7 @@
 #include "handle.h"
 #include "init.h"
 #include "io.h"
+#include "libfrog/handle_priv.h"
 
 static cmdinfo_t parent_cmd;
 static char *mntpt;
@@ -205,12 +206,8 @@ parent_f(
                        return 0;
                }
 
-               memcpy(&handle, hanp, sizeof(handle));
-               handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
-                               sizeof(handle.ha_fid.fid_len);
-               handle.ha_fid.fid_pad = 0;
-               handle.ha_fid.fid_ino = ino;
-               handle.ha_fid.fid_gen = gen;
+               handle_from_fshandle(&handle, hanp, hlen);
+               handle_from_inogen(&handle, ino, gen);
        } else if (optind != argc) {
                return command_usage(&parent_cmd);
        }

diff --git a/libfrog/Makefile b/libfrog/Makefile
index 4da427789411a6031d2019888107f99fd2bcbfae..fc7e506d96bbad7549023dbb2f149c6411d09821 100644 (file)
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -53,6 +53,7 @@ fsgeom.h \
 fsproperties.h \
 fsprops.h \
 getparents.h \
+handle_priv.h \
 histogram.h \
 logging.h \
 paths.h \

diff --git a/libfrog/handle_priv.h b/libfrog/handle_priv.h
new file mode 100644 (file)
index 0000000..8c3634c
--- /dev/null
+++ b/libfrog/handle_priv.h
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Oracle.  All Rights Reserved.
+ * Author: Darrick J. Wong <djwong@kernel.org>
+ */
+#ifndef __LIBFROG_HANDLE_PRIV_H__
+#define __LIBFROG_HANDLE_PRIV_H__
+
+/*
+ * Private helpers to construct an xfs_handle without publishing those details
+ * in the public libhandle header files.
+ */
+
+/*
+ * Fills out the fsid part of a handle.  This does not initialize the fid part
+ * of the handle; use either of the two functions below.
+ */
+static inline void
+handle_from_fshandle(
+       struct xfs_handle       *handle,
+       const void              *fshandle,
+       size_t                  fshandle_len)
+{
+       ASSERT(fshandle_len == sizeof(xfs_fsid_t));
+
+       memcpy(&handle->ha_fsid, fshandle, sizeof(handle->ha_fsid));
+       handle->ha_fid.fid_len = sizeof(xfs_fid_t) -
+                       sizeof(handle->ha_fid.fid_len);
+       handle->ha_fid.fid_pad = 0;
+       handle->ha_fid.fid_ino = 0;
+       handle->ha_fid.fid_gen = 0;
+}
+
+/* Fill out the fid part of a handle from raw components. */
+static inline void
+handle_from_inogen(
+       struct xfs_handle       *handle,
+       uint64_t                ino,
+       uint32_t                gen)
+{
+       handle->ha_fid.fid_ino = ino;
+       handle->ha_fid.fid_gen = gen;
+}
+
+/* Fill out the fid part of a handle. */
+static inline void
+handle_from_bulkstat(
+       struct xfs_handle               *handle,
+       const struct xfs_bulkstat       *bstat)
+{
+       handle->ha_fid.fid_ino = bstat->bs_ino;
+       handle->ha_fid.fid_gen = bstat->bs_gen;
+}
+
+#endif /* __LIBFROG_HANDLE_PRIV_H__ */

diff --git a/scrub/common.c b/scrub/common.c
index f86546556f46dd6731a98c72dbf8f24c6bac8a97..6eb3c026dc5ac97421c30b1298ad1e7be183091f 100644 (file)
--- a/scrub/common.c
+++ b/scrub/common.c
@@ -10,6 +10,7 @@
 #include "platform_defs.h"
 #include "libfrog/paths.h"
 #include "libfrog/getparents.h"
+#include "libfrog/handle_priv.h"
 #include "xfs_scrub.h"
 #include "common.h"
 #include "progress.h"
@@ -414,12 +415,8 @@ scrub_render_ino_descr(
        if (ctx->mnt.fsgeom.flags & XFS_FSOP_GEOM_FLAGS_PARENT) {
                struct xfs_handle handle;
 
-               memcpy(&handle.ha_fsid, ctx->fshandle, sizeof(handle.ha_fsid));
-               handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
-                               sizeof(handle.ha_fid.fid_len);
-               handle.ha_fid.fid_pad = 0;
-               handle.ha_fid.fid_ino = ino;
-               handle.ha_fid.fid_gen = gen;
+               handle_from_fshandle(&handle, ctx->fshandle, ctx->fshandle_len);
+               handle_from_inogen(&handle, ino, gen);
 
                ret = handle_to_path(&handle, sizeof(struct xfs_handle), 4096,
                                buf, buflen);

diff --git a/scrub/inodes.c b/scrub/inodes.c
index 3fe759e8f4867d98ff747cadee1ab174221b3ee4..2b492a634ea3b25a576c6a891374fb98916fbb1d 100644 (file)
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -19,6 +19,7 @@
 #include "descr.h"
 #include "libfrog/fsgeom.h"
 #include "libfrog/bulkstat.h"
+#include "libfrog/handle_priv.h"
 
 /*
  * Iterate a range of inodes.
@@ -209,7 +210,7 @@ scan_ag_bulkstat(
        xfs_agnumber_t          agno,
        void                    *arg)
 {
-       struct xfs_handle       handle = { };
+       struct xfs_handle       handle;
        struct scrub_ctx        *ctx = (struct scrub_ctx *)wq->wq_ctx;
        struct scan_ichunk      *ichunk = arg;
        struct xfs_inumbers_req *ireq = ichunk_to_inumbers(ichunk);
@@ -225,12 +226,7 @@ scan_ag_bulkstat(
        DEFINE_DESCR(dsc_inumbers, ctx, render_inumbers_from_agno);
 
        descr_set(&dsc_inumbers, &agno);
-
-       memcpy(&handle.ha_fsid, ctx->fshandle, sizeof(handle.ha_fsid));
-       handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
-                       sizeof(handle.ha_fid.fid_len);
-       handle.ha_fid.fid_pad = 0;
-
+       handle_from_fshandle(&handle, ctx->fshandle, ctx->fshandle_len);
 retry:
        bulkstat_for_inumbers(ctx, &dsc_inumbers, inumbers, breq);
 
@@ -244,8 +240,7 @@ retry:
                        continue;
 
                descr_set(&dsc_bulkstat, bs);
-               handle.ha_fid.fid_ino = scan_ino;
-               handle.ha_fid.fid_gen = bs->bs_gen;
+               handle_from_bulkstat(&handle, bs);
                error = si->fn(ctx, &handle, bs, si->arg);
                switch (error) {
                case 0:

diff --git a/scrub/phase5.c b/scrub/phase5.c
index 22a22915dbc68dd5707e541a0c5aaf25b720379c..6460d00f30f4bd5aa4af13baaa5d84f530241174 100644 (file)
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -18,6 +18,7 @@
 #include "libfrog/bitmap.h"
 #include "libfrog/bulkstat.h"
 #include "libfrog/fakelibattr.h"
+#include "libfrog/handle_priv.h"
 #include "xfs_scrub.h"
 #include "common.h"
 #include "inodes.h"
@@ -474,9 +475,7 @@ retry_deferred_inode(
        if (error)
                return error;
 
-       handle->ha_fid.fid_ino = bstat.bs_ino;
-       handle->ha_fid.fid_gen = bstat.bs_gen;
-
+       handle_from_bulkstat(handle, &bstat);
        return check_inode_names(ncs->ctx, handle, &bstat, ncs);
 }
 
@@ -487,16 +486,13 @@ retry_deferred_inode_range(
        uint64_t                len,
        void                    *arg)
 {
-       struct xfs_handle       handle = { };
+       struct xfs_handle       handle;
        struct ncheck_state     *ncs = arg;
        struct scrub_ctx        *ctx = ncs->ctx;
        uint64_t                i;
        int                     error;
 
-       memcpy(&handle.ha_fsid, ctx->fshandle, sizeof(handle.ha_fsid));
-       handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
-                       sizeof(handle.ha_fid.fid_len);
-       handle.ha_fid.fid_pad = 0;
+       handle_from_fshandle(&handle, ctx->fshandle, ctx->fshandle_len);
 
        for (i = 0; i < len; i++) {
                error = retry_deferred_inode(ncs, &handle, ino + i);

diff --git a/spaceman/health.c b/spaceman/health.c
index 4281589324cd4446c8a3708dcb1b5cb960ac3191..0d2767df424f270744a08a0aecbbb78076e66235 100644 (file)
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -14,6 +14,7 @@
 #include "libfrog/bulkstat.h"
 #include "space.h"
 #include "libfrog/getparents.h"
+#include "libfrog/handle_priv.h"
 
 static cmdinfo_t health_cmd;
 static unsigned long long reported;
@@ -317,12 +318,8 @@ report_inode(
            (file->xfd.fsgeom.flags & XFS_FSOP_GEOM_FLAGS_PARENT)) {
                struct xfs_handle handle;
 
-               memcpy(&handle.ha_fsid, file->fshandle, sizeof(handle.ha_fsid));
-               handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
-                               sizeof(handle.ha_fid.fid_len);
-               handle.ha_fid.fid_pad = 0;
-               handle.ha_fid.fid_ino = bs->bs_ino;
-               handle.ha_fid.fid_gen = bs->bs_gen;
+               handle_from_fshandle(&handle, file->fshandle, file->fshandle_len);
+               handle_from_bulkstat(&handle, bs);
 
                ret = handle_to_path(&handle, sizeof(struct xfs_handle), 0,
                                descr, sizeof(descr) - 1);