[1/2] misc: clean up code around attr_list_by_handle calls

Commit Message

Darrick J. Wong Sept. 19, 2024, 11:53 p.m. UTC

From: Darrick J. Wong <[e-mail]>

Reduce stack usage of the attr_list_by_handle calls by allocating the
buffers dynamically.  Remove some redundant bits while we're at it.

Signed-off-by: Darrick J. Wong <[e-mail]>
---
 libfrog/fsprops.c |   10 ++++++----
 scrub/phase5.c    |   46 +++++++++++++++++++++++++++++-----------------
 2 files changed, 35 insertions(+), 21 deletions(-)

Comments

Christoph Hellwig Sept. 20, 2024, 11:26 a.m. UTC | #1

Looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Patch

diff --git a/libfrog/fsprops.c b/libfrog/fsprops.c
index 88046b7a0738..a25c2726cd58 100644
--- a/libfrog/fsprops.c
+++ b/libfrog/fsprops.c
@@ -69,13 +69,14 @@  fsprops_walk_names(
 	void			*priv)
 {
 	struct attrlist_cursor	cur = { };
-	char			attrbuf[XFS_XATTR_LIST_MAX];
-	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist		*attrlist;
 	int			ret;
 
-	memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
+	attrlist = calloc(XFS_XATTR_LIST_MAX, 1);
+	if (!attrlist)
+		return -1;
 
-	while ((ret = attr_list_by_handle(fph->hanp, fph->hlen, attrbuf,
+	while ((ret = attr_list_by_handle(fph->hanp, fph->hlen, attrlist,
 				XFS_XATTR_LIST_MAX, XFS_IOC_ATTR_ROOT,
 				&cur)) == 0) {
 		unsigned int	i;
@@ -96,6 +97,7 @@  fsprops_walk_names(
 			break;
 	}
 
+	free(attrlist);
 	return ret;
 }
 
diff --git a/scrub/phase5.c b/scrub/phase5.c
index f6c295c64ada..d298d628a998 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -190,30 +190,40 @@  check_xattr_ns_names(
 	struct xfs_bulkstat		*bstat,
 	const struct attrns_decode	*attr_ns)
 {
-	struct attrlist_cursor		cur;
-	char				attrbuf[XFS_XATTR_LIST_MAX];
-	char				keybuf[XATTR_NAME_MAX + 1];
-	struct attrlist			*attrlist = (struct attrlist *)attrbuf;
-	struct attrlist_ent		*ent;
+	struct attrlist_cursor		cur = { };
+	char				*keybuf;
+	struct attrlist			*attrlist;
 	struct unicrash			*uc = NULL;
 	int				i;
 	int				error;
 
+	attrlist = calloc(XFS_XATTR_LIST_MAX, 1);
+	if (!attrlist) {
+		error = errno;
+		str_errno(ctx, descr_render(dsc));
+		return error;
+	}
+
+	keybuf = calloc(XATTR_NAME_MAX + 1, 1);
+	if (!keybuf) {
+		error = errno;
+		str_errno(ctx, descr_render(dsc));
+		goto out_attrlist;
+	}
+
 	error = unicrash_xattr_init(&uc, ctx, bstat);
 	if (error) {
 		str_liberror(ctx, error, descr_render(dsc));
-		return error;
+		goto out_keybuf;
 	}
 
-	memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
-	memset(&cur, 0, sizeof(cur));
-	memset(keybuf, 0, XATTR_NAME_MAX + 1);
-	error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
-			XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
-	while (!error) {
+	while ((error = attr_list_by_handle(handle, sizeof(*handle), attrlist,
+				XFS_XATTR_LIST_MAX, attr_ns->flags,
+				&cur)) == 0) {
 		/* Examine the xattrs. */
 		for (i = 0; i < attrlist->al_count; i++) {
-			ent = ATTR_ENTRY(attrlist, i);
+			struct attrlist_ent	*ent = ATTR_ENTRY(attrlist, i);
+
 			snprintf(keybuf, XATTR_NAME_MAX, "%s.%s", attr_ns->name,
 					ent->a_name);
 			if (uc)
@@ -225,14 +235,12 @@  check_xattr_ns_names(
 						keybuf);
 			if (error) {
 				str_liberror(ctx, error, descr_render(dsc));
-				goto out;
+				goto out_uc;
 			}
 		}
 
 		if (!attrlist->al_more)
 			break;
-		error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
-				XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
 	}
 	if (error) {
 		if (errno == ESTALE)
@@ -241,8 +249,12 @@  check_xattr_ns_names(
 		if (errno)
 			str_errno(ctx, descr_render(dsc));
 	}
-out:
+out_uc:
 	unicrash_free(uc);
+out_keybuf:
+	free(keybuf);
+out_attrlist:
+	free(attrlist);
 	return error;
 }