[2/3] libfrog: remove libattr dependency

Commit Message

Carlos Maiolino Aug. 27, 2024, 11:50 a.m. UTC

From: Carlos Maiolino <[e-mail]>

Get rid of libfrog's libattr dependency, and move the needed local
definitions to a new header - libfrog/attr.h

We could keep the ATTR_ENTRY definition local to fsprops.h, but we'll
add more content to it in the next patch.

Signed-off-by: Carlos Maiolino <[e-mail]>
---
 libfrog/Makefile  |  7 ++-----
 libfrog/attr.h    | 18 ++++++++++++++++++
 libfrog/fsprops.c |  7 +++----
 3 files changed, 23 insertions(+), 9 deletions(-)
 create mode 100644 libfrog/attr.h

Comments

Christoph Hellwig Aug. 27, 2024, 12:15 p.m. UTC | #1

> + * We are redifining here so we don't need to keep libattr as a dependency anymore

> +#define ATTR_ENTRY(buffer, index)		\

Maybe add a XFS_ prefix to distinguish this from the attrlist one
and match the other xfs_ prefixes?

> +	((struct xfs_attrlist_ent *)		\
> +	 &((char *)buffer)[ ((struct xfs_attrlist *)(buffer))->al_offset[index] ])

And maybe this really should be an inline function as well.

> +	struct xfs_attrlist		*attrlist = (struct xfs_attrlist *)attrbuf;

Overly long line

Otherwise this looks good.

Carlos Maiolino Aug. 27, 2024, 12:24 p.m. UTC | #2

On Tue, Aug 27, 2024 at 05:15:08AM GMT, Christoph Hellwig wrote:
> > + * We are redifining here so we don't need to keep libattr as a dependency anymore
> 
> > +#define ATTR_ENTRY(buffer, index)		\
> 
> Maybe add a XFS_ prefix to distinguish this from the attrlist one
> and match the other xfs_ prefixes?

Sounds good, will update it.

> 
> > +	((struct xfs_attrlist_ent *)		\
> > +	 &((char *)buffer)[ ((struct xfs_attrlist *)(buffer))->al_offset[index] ])
> 
> And maybe this really should be an inline function as well.
> 
> > +	struct xfs_attrlist		*attrlist = (struct xfs_attrlist *)attrbuf;
> 
> Overly long line
> 
> Otherwise this looks good.

Thanks for the review, I'll wait for darrick to take a look and will send a V2

Carlos

Patch

diff --git a/libfrog/Makefile b/libfrog/Makefile
index acddc894e..8581c146b 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -21,6 +21,7 @@  crc32.c \
 file_exchange.c \
 fsgeom.c \
 fsproperties.c \
+fsprops.c \
 getparents.c \
 histogram.c \
 list_sort.c \
@@ -49,6 +50,7 @@  div64.h \
 file_exchange.h \
 fsgeom.h \
 fsproperties.h \
+fsprops.h \
 getparents.h \
 histogram.h \
 logging.h \
@@ -62,11 +64,6 @@  workqueue.h
 
 LSRCFILES += gen_crc32table.c
 
-ifeq ($(HAVE_LIBATTR),yes)
-CFILES+=fsprops.c
-HFILES+=fsprops.h
-endif
-
 LDIRT = gen_crc32table crc32table.h
 
 default: ltdepend $(LTLIBRARY)
diff --git a/libfrog/attr.h b/libfrog/attr.h
new file mode 100644
index 000000000..9110499f2
--- /dev/null
+++ b/libfrog/attr.h
@@ -0,0 +1,18 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2024 Red Hat, Inc.  All Rights Reserved.
+ * Author: Carlos Maiolino <[e-mail]>
+ */
+#ifndef __LIBFROG_ATTR_H__
+#define __LIBFROG_ATTR_H__
+
+/*
+ * Those definitions come from libattr
+ *
+ * We are redifining here so we don't need to keep libattr as a dependency anymore
+ */
+#define ATTR_ENTRY(buffer, index)		\
+	((struct xfs_attrlist_ent *)		\
+	 &((char *)buffer)[ ((struct xfs_attrlist *)(buffer))->al_offset[index] ])
+
+#endif /* __LIBFROG_ATTR_H__ */
diff --git a/libfrog/fsprops.c b/libfrog/fsprops.c
index 05a584a56..ea47c66ed 100644
--- a/libfrog/fsprops.c
+++ b/libfrog/fsprops.c
@@ -10,8 +10,7 @@ 
 #include "libfrog/bulkstat.h"
 #include "libfrog/fsprops.h"
 #include "libfrog/fsproperties.h"
-
-#include <attr/attributes.h>
+#include "libfrog/attr.h"
 
 /*
  * Given an xfd and a mount table path, get us the handle for the root dir so
@@ -70,7 +69,7 @@  fsprops_walk_names(
 {
 	struct xfs_attrlist_cursor	cur = { };
 	char				attrbuf[XFS_XATTR_LIST_MAX];
-	struct attrlist			*attrlist = (struct attrlist *)attrbuf;
+	struct xfs_attrlist		*attrlist = (struct xfs_attrlist *)attrbuf;
 	int				ret;
 
 	memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
@@ -81,7 +80,7 @@  fsprops_walk_names(
 		unsigned int	i;
 
 		for (i = 0; i < attrlist->al_count; i++) {
-			struct attrlist_ent	*ent = ATTR_ENTRY(attrlist, i);
+			struct xfs_attrlist_ent	*ent = ATTR_ENTRY(attrlist, i);
 			const char		*p =
 				attr_name_to_fsprop_name(ent->a_name);