[PATCH 0/4] Clean up xfs_attr_sf_entry

[PATCH 0/4] Clean up xfs_attr_sf_entry

[Carlos Maiolino]

Hi,

this series has been suggested by Eric, and it's intended as a small clean up
for xfS_attr_sf_entry usage.

First patch changes the nameval array definition from 1 element to a
variable-size array. The array is already being used as a variable size array,
but by now, we need to subtract 1 from every time we use it.

Second patch just convert some macros to inline functions.

The remaining two patches are just 2 typedef cleanups that I think it's
appropriate since I'm already touching this code. I opted to leave the typedef
cleanups by last, since, if by any reason anybody think it's not worth it, both
patches can be simply discarded without needing to change any of the first 2.

All patches survived a few xfstests runs and the reproducer Eric shared on his
previous patch:

#touch file
#setfatt -n user.a file


Comments?

Cheers

Carlos Maiolino (4):
  xfs: Use variable-size array for nameval in xfs_attr_sf_entry
  xfs: Convert xfs_attr_sf macros to inline functions
  xfs: remove typedef xfs_attr_sf_entry_t
  xfs: Remove typedef xfs_attr_shortform_t

 fs/xfs/libxfs/xfs_attr.c      | 15 ++++++++++---
 fs/xfs/libxfs/xfs_attr_leaf.c | 42 +++++++++++++++++------------------
 fs/xfs/libxfs/xfs_attr_sf.h   | 27 +++++++++++++---------
 fs/xfs/libxfs/xfs_da_format.h |  6 ++---
 fs/xfs/xfs_attr_list.c        |  6 ++---
 fs/xfs/xfs_ondisk.h           | 12 +++++-----
 6 files changed, 60 insertions(+), 48 deletions(-)

-- 
2.26.2

[PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry

[Carlos Maiolino]

nameval is a variable-size array, so, define it as it, and remove all
the -1 magic number subtractions.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++----
 fs/xfs/libxfs/xfs_attr_sf.h   | 6 +++---
 fs/xfs/libxfs/xfs_da_format.h | 2 +-
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 305d4bc073370..7bbc97e0e4d4a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -992,7 +992,7 @@ xfs_attr_shortform_allfit(
 			return 0;
 		if (be16_to_cpu(name_loc->valuelen) >= XFS_ATTR_SF_ENTSIZE_MAX)
 			return 0;
-		bytes += sizeof(struct xfs_attr_sf_entry) - 1
+		bytes += sizeof(struct xfs_attr_sf_entry)
 				+ name_loc->namelen
 				+ be16_to_cpu(name_loc->valuelen);
 	}
@@ -1036,10 +1036,8 @@ xfs_attr_shortform_verify(
 		 * struct xfs_attr_sf_entry has a variable length.
 		 * Check the fixed-offset parts of the structure are
 		 * within the data buffer.
-		 * xfs_attr_sf_entry is defined with a 1-byte variable
-		 * array at the end, so we must subtract that off.
 		 */
-		if (((char *)sfep + sizeof(*sfep) - 1) >= endp)
+		if (((char *)sfep + sizeof(*sfep)) >= endp)
 			return __this_address;
 
 		/* Don't allow names with known bad length. */
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index bb004fb7944a7..d93012a0be4d0 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -28,11 +28,11 @@ typedef struct xfs_attr_sf_sort {
 } xfs_attr_sf_sort_t;
 
 #define XFS_ATTR_SF_ENTSIZE_BYNAME(nlen,vlen)	/* space name/value uses */ \
-	(((int)sizeof(xfs_attr_sf_entry_t)-1 + (nlen)+(vlen)))
+	(((int)sizeof(xfs_attr_sf_entry_t) + (nlen)+(vlen)))
 #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
-	((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
+	(1 << (NBBY*(int)sizeof(uint8_t)))
 #define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
-	((int)sizeof(xfs_attr_sf_entry_t)-1 + (sfep)->namelen+(sfep)->valuelen)
+	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)
 #define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
 	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
 #define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 059ac108b1b39..e86185a1165b3 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -589,7 +589,7 @@ typedef struct xfs_attr_shortform {
 		uint8_t namelen;	/* actual length of name (no NULL) */
 		uint8_t valuelen;	/* actual length of value (no NULL) */
 		uint8_t flags;	/* flags bits (see xfs_attr_leaf.h) */
-		uint8_t nameval[1];	/* name & value bytes concatenated */
+		uint8_t nameval[];	/* name & value bytes concatenated */
 	} list[1];			/* variable sized array */
 } xfs_attr_shortform_t;
 
-- 
2.26.2

[PATCH 2/4] xfs: Convert xfs_attr_sf macros to inline functions

[Carlos Maiolino]

xfs_attr_sf_totsize() requires access to xfs_inode structure, so, once
xfs_attr_shortform_addname() is its only user, move it to xfs_attr.c
instead of playing with more #includes.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/libxfs/xfs_attr.c      | 15 ++++++++++++---
 fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
 fs/xfs/libxfs/xfs_attr_sf.h   | 23 ++++++++++++++---------
 fs/xfs/xfs_attr_list.c        |  4 ++--
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 2e055c079f397..2b48fdb394e80 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -428,7 +428,7 @@ xfs_attr_set(
 		 */
 		if (XFS_IFORK_Q(dp) == 0) {
 			int sf_size = sizeof(struct xfs_attr_sf_hdr) +
-				XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen,
+				xfs_attr_sf_entsize_byname(args->namelen,
 						args->valuelen);
 
 			error = xfs_bmap_add_attrfork(dp, sf_size, rsvd);
@@ -523,6 +523,15 @@ xfs_attr_set(
  * External routines when attribute list is inside the inode
  *========================================================================*/
 
+static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
+
+	xfs_attr_shortform_t *sf =
+		(xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
+
+	return (be16_to_cpu(sf->hdr.totsize));
+
+}
+
 /*
  * Add a name to the shortform attribute list structure
  * This is the external routine.
@@ -555,8 +564,8 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
 	    args->valuelen >= XFS_ATTR_SF_ENTSIZE_MAX)
 		return -ENOSPC;
 
-	newsize = XFS_ATTR_SF_TOTSIZE(args->dp);
-	newsize += XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
+	newsize = xfs_attr_sf_totsize(args->dp);
+	newsize += xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
 
 	forkoff = xfs_attr_shortform_bytesfit(args->dp, newsize);
 	if (!forkoff)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 7bbc97e0e4d4a..a8a4e21d19726 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -684,9 +684,9 @@ xfs_attr_sf_findname(
 	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
 	sfe = &sf->list[0];
 	end = sf->hdr.count;
-	for (i = 0; i < end; sfe = XFS_ATTR_SF_NEXTENTRY(sfe),
+	for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
 			     base += size, i++) {
-		size = XFS_ATTR_SF_ENTSIZE(sfe);
+		size = xfs_attr_sf_entsize(sfe);
 		if (!xfs_attr_match(args, sfe->namelen, sfe->nameval,
 				    sfe->flags))
 			continue;
@@ -733,7 +733,7 @@ xfs_attr_shortform_add(
 		ASSERT(0);
 
 	offset = (char *)sfe - (char *)sf;
-	size = XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
+	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
 	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
 	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
 	sfe = (xfs_attr_sf_entry_t *)((char *)sf + offset);
@@ -792,7 +792,7 @@ xfs_attr_shortform_remove(
 	error = xfs_attr_sf_findname(args, &sfe, &base);
 	if (error != -EEXIST)
 		return error;
-	size = XFS_ATTR_SF_ENTSIZE(sfe);
+	size = xfs_attr_sf_entsize(sfe);
 
 	/*
 	 * Fix up the attribute fork data, covering the hole
@@ -849,7 +849,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
 	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
-				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
+				sfe = xfs_attr_sf_nextentry(sfe), i++) {
 		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
 				sfe->flags))
 			return -EEXIST;
@@ -876,7 +876,7 @@ xfs_attr_shortform_getvalue(
 	sf = (xfs_attr_shortform_t *)args->dp->i_afp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
-				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
+				sfe = xfs_attr_sf_nextentry(sfe), i++) {
 		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
 				sfe->flags))
 			return xfs_attr_copy_value(args,
@@ -951,7 +951,7 @@ xfs_attr_shortform_to_leaf(
 		ASSERT(error != -ENOSPC);
 		if (error)
 			goto out;
-		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
+		sfe = xfs_attr_sf_nextentry(sfe);
 	}
 	error = 0;
 	*leaf_bp = bp;
@@ -1049,7 +1049,7 @@ xfs_attr_shortform_verify(
 		 * within the data buffer.  The next entry starts after the
 		 * name component, so nextentry is an acceptable test.
 		 */
-		next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
+		next_sfep = xfs_attr_sf_nextentry(sfep);
 		if ((char *)next_sfep > endp)
 			return __this_address;
 
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index d93012a0be4d0..48906c5196505 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -27,16 +27,21 @@ typedef struct xfs_attr_sf_sort {
 	unsigned char	*name;		/* name value, pointer into buffer */
 } xfs_attr_sf_sort_t;
 
-#define XFS_ATTR_SF_ENTSIZE_BYNAME(nlen,vlen)	/* space name/value uses */ \
-	(((int)sizeof(xfs_attr_sf_entry_t) + (nlen)+(vlen)))
 #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
 	(1 << (NBBY*(int)sizeof(uint8_t)))
-#define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
-	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)
-#define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
-	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
-#define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
-	(be16_to_cpu(((xfs_attr_shortform_t *)	\
-		((dp)->i_afp->if_u1.if_data))->hdr.totsize))
 
+static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
+	return sizeof(xfs_attr_sf_entry_t) + nlen + vlen;
+}
+
+/* space an entry uses */
+static inline int xfs_attr_sf_entsize(xfs_attr_sf_entry_t *sfep) {
+	return sizeof(xfs_attr_sf_entry_t) + sfep->namelen + sfep->valuelen;
+}
+
+static inline xfs_attr_sf_entry_t *
+xfs_attr_sf_nextentry(xfs_attr_sf_entry_t *sfep) {
+	return (xfs_attr_sf_entry_t *)((char *)(sfep) +
+				       xfs_attr_sf_entsize(sfep));
+}
 #endif	/* __XFS_ATTR_SF_H__ */
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 50f922cad91a4..fbe5574f08930 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -96,7 +96,7 @@ xfs_attr_shortform_list(
 			 */
 			if (context->seen_enough)
 				break;
-			sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
+			sfe = xfs_attr_sf_nextentry(sfe);
 		}
 		trace_xfs_attr_list_sf_all(context);
 		return 0;
@@ -136,7 +136,7 @@ xfs_attr_shortform_list(
 		/* These are bytes, and both on-disk, don't endian-flip */
 		sbp->valuelen = sfe->valuelen;
 		sbp->flags = sfe->flags;
-		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
+		sfe = xfs_attr_sf_nextentry(sfe);
 		sbp++;
 		nsbuf++;
 	}
-- 
2.26.2

[PATCH 3/4] xfs: remove typedef xfs_attr_sf_entry_t

[Carlos Maiolino]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c |  4 ++--
 fs/xfs/libxfs/xfs_attr_sf.h   | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index a8a4e21d19726..bcc76ff298646 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -736,7 +736,7 @@ xfs_attr_shortform_add(
 	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
 	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
 	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
-	sfe = (xfs_attr_sf_entry_t *)((char *)sf + offset);
+	sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
 
 	sfe->namelen = args->namelen;
 	sfe->valuelen = args->valuelen;
@@ -838,7 +838,7 @@ int
 xfs_attr_shortform_lookup(xfs_da_args_t *args)
 {
 	xfs_attr_shortform_t *sf;
-	xfs_attr_sf_entry_t *sfe;
+	struct xfs_attr_sf_entry *sfe;
 	int i;
 	struct xfs_ifork *ifp;
 
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index 48906c5196505..6b09a010940ea 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -13,7 +13,6 @@
  * to fit into the literal area of the inode.
  */
 typedef struct xfs_attr_sf_hdr xfs_attr_sf_hdr_t;
-typedef struct xfs_attr_sf_entry xfs_attr_sf_entry_t;
 
 /*
  * We generate this then sort it, attr_list() must return things in hash-order.
@@ -31,17 +30,18 @@ typedef struct xfs_attr_sf_sort {
 	(1 << (NBBY*(int)sizeof(uint8_t)))
 
 static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
-	return sizeof(xfs_attr_sf_entry_t) + nlen + vlen;
+	return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
 }
 
 /* space an entry uses */
-static inline int xfs_attr_sf_entsize(xfs_attr_sf_entry_t *sfep) {
-	return sizeof(xfs_attr_sf_entry_t) + sfep->namelen + sfep->valuelen;
+static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
+	return sizeof(struct xfs_attr_sf_entry) +
+		sfep->namelen + sfep->valuelen;
 }
 
-static inline xfs_attr_sf_entry_t *
-xfs_attr_sf_nextentry(xfs_attr_sf_entry_t *sfep) {
-	return (xfs_attr_sf_entry_t *)((char *)(sfep) +
+static inline struct xfs_attr_sf_entry *
+xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep) {
+	return (struct xfs_attr_sf_entry *)((char *)(sfep) +
 				       xfs_attr_sf_entsize(sfep));
 }
 #endif	/* __XFS_ATTR_SF_H__ */
-- 
2.26.2

[PATCH 4/4] xfs: Remove typedef xfs_attr_shortform_t

[Carlos Maiolino]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/libxfs/xfs_attr.c      |  4 ++--
 fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
 fs/xfs/libxfs/xfs_da_format.h |  4 ++--
 fs/xfs/xfs_attr_list.c        |  2 +-
 fs/xfs/xfs_ondisk.h           | 12 ++++++------
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 2b48fdb394e80..0468ead236924 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -525,8 +525,8 @@ xfs_attr_set(
 
 static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
 
-	xfs_attr_shortform_t *sf =
-		(xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
+	struct xfs_attr_shortform *sf =
+		(struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
 
 	return (be16_to_cpu(sf->hdr.totsize));
 
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index bcc76ff298646..f64ab351b760c 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -728,14 +728,14 @@ xfs_attr_shortform_add(
 
 	ifp = dp->i_afp;
 	ASSERT(ifp->if_flags & XFS_IFINLINE);
-	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
 		ASSERT(0);
 
 	offset = (char *)sfe - (char *)sf;
 	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
 	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
-	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
 
 	sfe->namelen = args->namelen;
@@ -787,7 +787,7 @@ xfs_attr_shortform_remove(
 
 	dp = args->dp;
 	mp = dp->i_mount;
-	sf = (xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
 
 	error = xfs_attr_sf_findname(args, &sfe, &base);
 	if (error != -EEXIST)
@@ -837,7 +837,7 @@ xfs_attr_shortform_remove(
 int
 xfs_attr_shortform_lookup(xfs_da_args_t *args)
 {
-	xfs_attr_shortform_t *sf;
+	struct xfs_attr_shortform *sf;
 	struct xfs_attr_sf_entry *sfe;
 	int i;
 	struct xfs_ifork *ifp;
@@ -846,7 +846,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
 
 	ifp = args->dp->i_afp;
 	ASSERT(ifp->if_flags & XFS_IFINLINE);
-	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
 				sfe = xfs_attr_sf_nextentry(sfe), i++) {
@@ -873,7 +873,7 @@ xfs_attr_shortform_getvalue(
 	int			i;
 
 	ASSERT(args->dp->i_afp->if_flags == XFS_IFINLINE);
-	sf = (xfs_attr_shortform_t *)args->dp->i_afp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
 				sfe = xfs_attr_sf_nextentry(sfe), i++) {
@@ -908,12 +908,12 @@ xfs_attr_shortform_to_leaf(
 
 	dp = args->dp;
 	ifp = dp->i_afp;
-	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	size = be16_to_cpu(sf->hdr.totsize);
 	tmpbuffer = kmem_alloc(size, 0);
 	ASSERT(tmpbuffer != NULL);
 	memcpy(tmpbuffer, ifp->if_u1.if_data, size);
-	sf = (xfs_attr_shortform_t *)tmpbuffer;
+	sf = (struct xfs_attr_shortform *)tmpbuffer;
 
 	xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
 	xfs_bmap_local_to_extents_empty(args->trans, dp, XFS_ATTR_FORK);
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index e86185a1165b3..b40a4e80f5ee6 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -579,7 +579,7 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
 /*
  * Entries are packed toward the top as tight as possible.
  */
-typedef struct xfs_attr_shortform {
+struct xfs_attr_shortform {
 	struct xfs_attr_sf_hdr {	/* constant-structure header block */
 		__be16	totsize;	/* total bytes in shortform list */
 		__u8	count;	/* count of active entries */
@@ -591,7 +591,7 @@ typedef struct xfs_attr_shortform {
 		uint8_t flags;	/* flags bits (see xfs_attr_leaf.h) */
 		uint8_t nameval[];	/* name & value bytes concatenated */
 	} list[1];			/* variable sized array */
-} xfs_attr_shortform_t;
+};
 
 typedef struct xfs_attr_leaf_map {	/* RLE map of free bytes */
 	__be16	base;			  /* base of free region */
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index fbe5574f08930..8f8837fe21cf0 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -61,7 +61,7 @@ xfs_attr_shortform_list(
 	int				error = 0;
 
 	ASSERT(dp->i_afp != NULL);
-	sf = (xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
+	sf = (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
 	ASSERT(sf != NULL);
 	if (!sf->hdr.count)
 		return 0;
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index 5f04d8a5ab2a9..ad51c8eb447b1 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -84,12 +84,12 @@ xfs_check_ondisk_structs(void)
 	XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen,	8);
 	XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name,	9);
 	XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t,		40);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, hdr.totsize,	0);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, hdr.count,	2);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].namelen,	4);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].valuelen, 5);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].flags,	6);
-	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].nameval,	7);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count,	 2);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen,	4);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen,	5);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags,	6);
+	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval,	7);
 	XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t,			12);
 	XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t,			16);
 	XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t,		8);
-- 
2.26.2

Re: [PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry

[Eric Sandeen]

On 8/31/20 8:04 AM, Carlos Maiolino wrote:
>  #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
> -	((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
> +	(1 << (NBBY*(int)sizeof(uint8_t)))

This probably is not correct.  :)

This would cut the max size of attr (name+value) in half.

-Eric

Re: [PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry

[Eric Sandeen]

On 8/31/20 9:53 AM, Eric Sandeen wrote:
> On 8/31/20 8:04 AM, Carlos Maiolino wrote:
>>  #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
>> -	((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
>> +	(1 << (NBBY*(int)sizeof(uint8_t)))
> 
> This probably is not correct.  :)
> 
> This would cut the max size of attr (name+value) in half.

Whoops other way around.  ;)  this would double XFS_ATTR_SF_ENTSIZE_MAX.

In any case, just drop that change.

-Eric

Re: [PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry

[Darrick J. Wong]

On Mon, Aug 31, 2020 at 03:04:20PM +0200, Carlos Maiolino wrote:
> nameval is a variable-size array, so, define it as it, and remove all
> the -1 magic number subtractions.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++----
>  fs/xfs/libxfs/xfs_attr_sf.h   | 6 +++---
>  fs/xfs/libxfs/xfs_da_format.h | 2 +-
>  3 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 305d4bc073370..7bbc97e0e4d4a 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -992,7 +992,7 @@ xfs_attr_shortform_allfit(
>  			return 0;
>  		if (be16_to_cpu(name_loc->valuelen) >= XFS_ATTR_SF_ENTSIZE_MAX)
>  			return 0;
> -		bytes += sizeof(struct xfs_attr_sf_entry) - 1
> +		bytes += sizeof(struct xfs_attr_sf_entry)
>  				+ name_loc->namelen
>  				+ be16_to_cpu(name_loc->valuelen);
>  	}
> @@ -1036,10 +1036,8 @@ xfs_attr_shortform_verify(
>  		 * struct xfs_attr_sf_entry has a variable length.
>  		 * Check the fixed-offset parts of the structure are
>  		 * within the data buffer.
> -		 * xfs_attr_sf_entry is defined with a 1-byte variable
> -		 * array at the end, so we must subtract that off.
>  		 */
> -		if (((char *)sfep + sizeof(*sfep) - 1) >= endp)
> +		if (((char *)sfep + sizeof(*sfep)) >= endp)
>  			return __this_address;
>  
>  		/* Don't allow names with known bad length. */
> diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
> index bb004fb7944a7..d93012a0be4d0 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -28,11 +28,11 @@ typedef struct xfs_attr_sf_sort {
>  } xfs_attr_sf_sort_t;
>  
>  #define XFS_ATTR_SF_ENTSIZE_BYNAME(nlen,vlen)	/* space name/value uses */ \
> -	(((int)sizeof(xfs_attr_sf_entry_t)-1 + (nlen)+(vlen)))
> +	(((int)sizeof(xfs_attr_sf_entry_t) + (nlen)+(vlen)))
>  #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
> -	((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
> +	(1 << (NBBY*(int)sizeof(uint8_t)))

The maximum space for the name and value is still UINT8_MAX, right?
I don't think this should change to 256.

I also kind of wonder if this should also get changed to be more
direct:

#define XFS_ATTR_SF_ENTSIZE_MAX		(UINT8_MAX)

but it's working code, we could/should just leave it be...

>  #define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
> -	((int)sizeof(xfs_attr_sf_entry_t)-1 + (sfep)->namelen+(sfep)->valuelen)
> +	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)

Can this (and ENTSIZE_BYNAME) use struct_sizeof?

--D

>  #define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
>  	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
>  #define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index 059ac108b1b39..e86185a1165b3 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -589,7 +589,7 @@ typedef struct xfs_attr_shortform {
>  		uint8_t namelen;	/* actual length of name (no NULL) */
>  		uint8_t valuelen;	/* actual length of value (no NULL) */
>  		uint8_t flags;	/* flags bits (see xfs_attr_leaf.h) */
> -		uint8_t nameval[1];	/* name & value bytes concatenated */
> +		uint8_t nameval[];	/* name & value bytes concatenated */
>  	} list[1];			/* variable sized array */
>  } xfs_attr_shortform_t;
>  
> -- 
> 2.26.2
> 

Re: [PATCH 2/4] xfs: Convert xfs_attr_sf macros to inline functions

[Darrick J. Wong]

On Mon, Aug 31, 2020 at 03:04:21PM +0200, Carlos Maiolino wrote:
> xfs_attr_sf_totsize() requires access to xfs_inode structure, so, once
> xfs_attr_shortform_addname() is its only user, move it to xfs_attr.c
> instead of playing with more #includes.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

/me would have preferred you kill the typedef at the start of the series
instead of creating new functions with them and then changing them in
the next patch, but aside from that...

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c      | 15 ++++++++++++---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
>  fs/xfs/libxfs/xfs_attr_sf.h   | 23 ++++++++++++++---------
>  fs/xfs/xfs_attr_list.c        |  4 ++--
>  4 files changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 2e055c079f397..2b48fdb394e80 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -428,7 +428,7 @@ xfs_attr_set(
>  		 */
>  		if (XFS_IFORK_Q(dp) == 0) {
>  			int sf_size = sizeof(struct xfs_attr_sf_hdr) +
> -				XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen,
> +				xfs_attr_sf_entsize_byname(args->namelen,
>  						args->valuelen);
>  
>  			error = xfs_bmap_add_attrfork(dp, sf_size, rsvd);
> @@ -523,6 +523,15 @@ xfs_attr_set(
>   * External routines when attribute list is inside the inode
>   *========================================================================*/
>  
> +static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
> +
> +	xfs_attr_shortform_t *sf =
> +		(xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
> +
> +	return (be16_to_cpu(sf->hdr.totsize));
> +
> +}
> +
>  /*
>   * Add a name to the shortform attribute list structure
>   * This is the external routine.
> @@ -555,8 +564,8 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
>  	    args->valuelen >= XFS_ATTR_SF_ENTSIZE_MAX)
>  		return -ENOSPC;
>  
> -	newsize = XFS_ATTR_SF_TOTSIZE(args->dp);
> -	newsize += XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
> +	newsize = xfs_attr_sf_totsize(args->dp);
> +	newsize += xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
>  
>  	forkoff = xfs_attr_shortform_bytesfit(args->dp, newsize);
>  	if (!forkoff)
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 7bbc97e0e4d4a..a8a4e21d19726 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -684,9 +684,9 @@ xfs_attr_sf_findname(
>  	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
>  	sfe = &sf->list[0];
>  	end = sf->hdr.count;
> -	for (i = 0; i < end; sfe = XFS_ATTR_SF_NEXTENTRY(sfe),
> +	for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
>  			     base += size, i++) {
> -		size = XFS_ATTR_SF_ENTSIZE(sfe);
> +		size = xfs_attr_sf_entsize(sfe);
>  		if (!xfs_attr_match(args, sfe->namelen, sfe->nameval,
>  				    sfe->flags))
>  			continue;
> @@ -733,7 +733,7 @@ xfs_attr_shortform_add(
>  		ASSERT(0);
>  
>  	offset = (char *)sfe - (char *)sf;
> -	size = XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
> +	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
>  	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
>  	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
>  	sfe = (xfs_attr_sf_entry_t *)((char *)sf + offset);
> @@ -792,7 +792,7 @@ xfs_attr_shortform_remove(
>  	error = xfs_attr_sf_findname(args, &sfe, &base);
>  	if (error != -EEXIST)
>  		return error;
> -	size = XFS_ATTR_SF_ENTSIZE(sfe);
> +	size = xfs_attr_sf_entsize(sfe);
>  
>  	/*
>  	 * Fix up the attribute fork data, covering the hole
> @@ -849,7 +849,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
>  	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
>  	sfe = &sf->list[0];
>  	for (i = 0; i < sf->hdr.count;
> -				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
> +				sfe = xfs_attr_sf_nextentry(sfe), i++) {
>  		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
>  				sfe->flags))
>  			return -EEXIST;
> @@ -876,7 +876,7 @@ xfs_attr_shortform_getvalue(
>  	sf = (xfs_attr_shortform_t *)args->dp->i_afp->if_u1.if_data;
>  	sfe = &sf->list[0];
>  	for (i = 0; i < sf->hdr.count;
> -				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
> +				sfe = xfs_attr_sf_nextentry(sfe), i++) {
>  		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
>  				sfe->flags))
>  			return xfs_attr_copy_value(args,
> @@ -951,7 +951,7 @@ xfs_attr_shortform_to_leaf(
>  		ASSERT(error != -ENOSPC);
>  		if (error)
>  			goto out;
> -		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> +		sfe = xfs_attr_sf_nextentry(sfe);
>  	}
>  	error = 0;
>  	*leaf_bp = bp;
> @@ -1049,7 +1049,7 @@ xfs_attr_shortform_verify(
>  		 * within the data buffer.  The next entry starts after the
>  		 * name component, so nextentry is an acceptable test.
>  		 */
> -		next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
> +		next_sfep = xfs_attr_sf_nextentry(sfep);
>  		if ((char *)next_sfep > endp)
>  			return __this_address;
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
> index d93012a0be4d0..48906c5196505 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -27,16 +27,21 @@ typedef struct xfs_attr_sf_sort {
>  	unsigned char	*name;		/* name value, pointer into buffer */
>  } xfs_attr_sf_sort_t;
>  
> -#define XFS_ATTR_SF_ENTSIZE_BYNAME(nlen,vlen)	/* space name/value uses */ \
> -	(((int)sizeof(xfs_attr_sf_entry_t) + (nlen)+(vlen)))
>  #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
>  	(1 << (NBBY*(int)sizeof(uint8_t)))
> -#define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
> -	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)
> -#define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
> -	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
> -#define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
> -	(be16_to_cpu(((xfs_attr_shortform_t *)	\
> -		((dp)->i_afp->if_u1.if_data))->hdr.totsize))
>  
> +static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> +	return sizeof(xfs_attr_sf_entry_t) + nlen + vlen;
> +}
> +
> +/* space an entry uses */
> +static inline int xfs_attr_sf_entsize(xfs_attr_sf_entry_t *sfep) {
> +	return sizeof(xfs_attr_sf_entry_t) + sfep->namelen + sfep->valuelen;
> +}
> +
> +static inline xfs_attr_sf_entry_t *
> +xfs_attr_sf_nextentry(xfs_attr_sf_entry_t *sfep) {
> +	return (xfs_attr_sf_entry_t *)((char *)(sfep) +
> +				       xfs_attr_sf_entsize(sfep));
> +}
>  #endif	/* __XFS_ATTR_SF_H__ */
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 50f922cad91a4..fbe5574f08930 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -96,7 +96,7 @@ xfs_attr_shortform_list(
>  			 */
>  			if (context->seen_enough)
>  				break;
> -			sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> +			sfe = xfs_attr_sf_nextentry(sfe);
>  		}
>  		trace_xfs_attr_list_sf_all(context);
>  		return 0;
> @@ -136,7 +136,7 @@ xfs_attr_shortform_list(
>  		/* These are bytes, and both on-disk, don't endian-flip */
>  		sbp->valuelen = sfe->valuelen;
>  		sbp->flags = sfe->flags;
> -		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> +		sfe = xfs_attr_sf_nextentry(sfe);
>  		sbp++;
>  		nsbuf++;
>  	}
> -- 
> 2.26.2
> 

Re: [PATCH 3/4] xfs: remove typedef xfs_attr_sf_entry_t

[Darrick J. Wong]

On Mon, Aug 31, 2020 at 03:04:22PM +0200, Carlos Maiolino wrote:
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c |  4 ++--
>  fs/xfs/libxfs/xfs_attr_sf.h   | 14 +++++++-------
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index a8a4e21d19726..bcc76ff298646 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -736,7 +736,7 @@ xfs_attr_shortform_add(
>  	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
>  	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
>  	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> -	sfe = (xfs_attr_sf_entry_t *)((char *)sf + offset);
> +	sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
>  
>  	sfe->namelen = args->namelen;
>  	sfe->valuelen = args->valuelen;
> @@ -838,7 +838,7 @@ int
>  xfs_attr_shortform_lookup(xfs_da_args_t *args)
>  {
>  	xfs_attr_shortform_t *sf;
> -	xfs_attr_sf_entry_t *sfe;
> +	struct xfs_attr_sf_entry *sfe;
>  	int i;
>  	struct xfs_ifork *ifp;
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
> index 48906c5196505..6b09a010940ea 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -13,7 +13,6 @@
>   * to fit into the literal area of the inode.
>   */
>  typedef struct xfs_attr_sf_hdr xfs_attr_sf_hdr_t;
> -typedef struct xfs_attr_sf_entry xfs_attr_sf_entry_t;
>  
>  /*
>   * We generate this then sort it, attr_list() must return things in hash-order.
> @@ -31,17 +30,18 @@ typedef struct xfs_attr_sf_sort {
>  	(1 << (NBBY*(int)sizeof(uint8_t)))
>  
>  static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> -	return sizeof(xfs_attr_sf_entry_t) + nlen + vlen;
> +	return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
>  }
>  
>  /* space an entry uses */
> -static inline int xfs_attr_sf_entsize(xfs_attr_sf_entry_t *sfep) {
> -	return sizeof(xfs_attr_sf_entry_t) + sfep->namelen + sfep->valuelen;
> +static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
> +	return sizeof(struct xfs_attr_sf_entry) +
> +		sfep->namelen + sfep->valuelen;
>  }
>  
> -static inline xfs_attr_sf_entry_t *
> -xfs_attr_sf_nextentry(xfs_attr_sf_entry_t *sfep) {
> -	return (xfs_attr_sf_entry_t *)((char *)(sfep) +
> +static inline struct xfs_attr_sf_entry *
> +xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep) {
> +	return (struct xfs_attr_sf_entry *)((char *)(sfep) +
>  				       xfs_attr_sf_entsize(sfep));
>  }
>  #endif	/* __XFS_ATTR_SF_H__ */
> -- 
> 2.26.2
> 

Re: [PATCH 4/4] xfs: Remove typedef xfs_attr_shortform_t

[Darrick J. Wong]

On Mon, Aug 31, 2020 at 03:04:23PM +0200, Carlos Maiolino wrote:
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks straightforward,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c      |  4 ++--
>  fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
>  fs/xfs/libxfs/xfs_da_format.h |  4 ++--
>  fs/xfs/xfs_attr_list.c        |  2 +-
>  fs/xfs/xfs_ondisk.h           | 12 ++++++------
>  5 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 2b48fdb394e80..0468ead236924 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -525,8 +525,8 @@ xfs_attr_set(
>  
>  static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
>  
> -	xfs_attr_shortform_t *sf =
> -		(xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
> +	struct xfs_attr_shortform *sf =
> +		(struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
>  
>  	return (be16_to_cpu(sf->hdr.totsize));
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index bcc76ff298646..f64ab351b760c 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -728,14 +728,14 @@ xfs_attr_shortform_add(
>  
>  	ifp = dp->i_afp;
>  	ASSERT(ifp->if_flags & XFS_IFINLINE);
> -	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
>  	if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
>  		ASSERT(0);
>  
>  	offset = (char *)sfe - (char *)sf;
>  	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
>  	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
> -	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
>  	sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
>  
>  	sfe->namelen = args->namelen;
> @@ -787,7 +787,7 @@ xfs_attr_shortform_remove(
>  
>  	dp = args->dp;
>  	mp = dp->i_mount;
> -	sf = (xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
>  
>  	error = xfs_attr_sf_findname(args, &sfe, &base);
>  	if (error != -EEXIST)
> @@ -837,7 +837,7 @@ xfs_attr_shortform_remove(
>  int
>  xfs_attr_shortform_lookup(xfs_da_args_t *args)
>  {
> -	xfs_attr_shortform_t *sf;
> +	struct xfs_attr_shortform *sf;
>  	struct xfs_attr_sf_entry *sfe;
>  	int i;
>  	struct xfs_ifork *ifp;
> @@ -846,7 +846,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
>  
>  	ifp = args->dp->i_afp;
>  	ASSERT(ifp->if_flags & XFS_IFINLINE);
> -	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
>  	sfe = &sf->list[0];
>  	for (i = 0; i < sf->hdr.count;
>  				sfe = xfs_attr_sf_nextentry(sfe), i++) {
> @@ -873,7 +873,7 @@ xfs_attr_shortform_getvalue(
>  	int			i;
>  
>  	ASSERT(args->dp->i_afp->if_flags == XFS_IFINLINE);
> -	sf = (xfs_attr_shortform_t *)args->dp->i_afp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
>  	sfe = &sf->list[0];
>  	for (i = 0; i < sf->hdr.count;
>  				sfe = xfs_attr_sf_nextentry(sfe), i++) {
> @@ -908,12 +908,12 @@ xfs_attr_shortform_to_leaf(
>  
>  	dp = args->dp;
>  	ifp = dp->i_afp;
> -	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
>  	size = be16_to_cpu(sf->hdr.totsize);
>  	tmpbuffer = kmem_alloc(size, 0);
>  	ASSERT(tmpbuffer != NULL);
>  	memcpy(tmpbuffer, ifp->if_u1.if_data, size);
> -	sf = (xfs_attr_shortform_t *)tmpbuffer;
> +	sf = (struct xfs_attr_shortform *)tmpbuffer;
>  
>  	xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
>  	xfs_bmap_local_to_extents_empty(args->trans, dp, XFS_ATTR_FORK);
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index e86185a1165b3..b40a4e80f5ee6 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -579,7 +579,7 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
>  /*
>   * Entries are packed toward the top as tight as possible.
>   */
> -typedef struct xfs_attr_shortform {
> +struct xfs_attr_shortform {
>  	struct xfs_attr_sf_hdr {	/* constant-structure header block */
>  		__be16	totsize;	/* total bytes in shortform list */
>  		__u8	count;	/* count of active entries */
> @@ -591,7 +591,7 @@ typedef struct xfs_attr_shortform {
>  		uint8_t flags;	/* flags bits (see xfs_attr_leaf.h) */
>  		uint8_t nameval[];	/* name & value bytes concatenated */
>  	} list[1];			/* variable sized array */
> -} xfs_attr_shortform_t;
> +};
>  
>  typedef struct xfs_attr_leaf_map {	/* RLE map of free bytes */
>  	__be16	base;			  /* base of free region */
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index fbe5574f08930..8f8837fe21cf0 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -61,7 +61,7 @@ xfs_attr_shortform_list(
>  	int				error = 0;
>  
>  	ASSERT(dp->i_afp != NULL);
> -	sf = (xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
> +	sf = (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
>  	ASSERT(sf != NULL);
>  	if (!sf->hdr.count)
>  		return 0;
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 5f04d8a5ab2a9..ad51c8eb447b1 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -84,12 +84,12 @@ xfs_check_ondisk_structs(void)
>  	XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen,	8);
>  	XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name,	9);
>  	XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t,		40);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, hdr.totsize,	0);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, hdr.count,	2);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].namelen,	4);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].valuelen, 5);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].flags,	6);
> -	XFS_CHECK_OFFSET(xfs_attr_shortform_t, list[0].nameval,	7);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count,	 2);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen,	4);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen,	5);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags,	6);
> +	XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval,	7);
>  	XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t,			12);
>  	XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t,			16);
>  	XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t,		8);
> -- 
> 2.26.2
> 

Re: [PATCH 2/4] xfs: Convert xfs_attr_sf macros to inline functions

[Carlos Maiolino]

On Mon, Aug 31, 2020 at 08:34:05AM -0700, Darrick J. Wong wrote:
> On Mon, Aug 31, 2020 at 03:04:21PM +0200, Carlos Maiolino wrote:
> > xfs_attr_sf_totsize() requires access to xfs_inode structure, so, once
> > xfs_attr_shortform_addname() is its only user, move it to xfs_attr.c
> > instead of playing with more #includes.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> 
> /me would have preferred you kill the typedef at the start of the series
> instead of creating new functions with them and then changing them in
> the next patch, but aside from that...

Yeah, that was my preference too, but I thought "If for any reason, the typdef
cleanups are a bad idea, it will be just easier to drop the patches instead of
rebasing the important ones".

Thanks for the review!

> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> --D
> 
> > ---
> >  fs/xfs/libxfs/xfs_attr.c      | 15 ++++++++++++---
> >  fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
> >  fs/xfs/libxfs/xfs_attr_sf.h   | 23 ++++++++++++++---------
> >  fs/xfs/xfs_attr_list.c        |  4 ++--
> >  4 files changed, 36 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index 2e055c079f397..2b48fdb394e80 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
> > @@ -428,7 +428,7 @@ xfs_attr_set(
> >  		 */
> >  		if (XFS_IFORK_Q(dp) == 0) {
> >  			int sf_size = sizeof(struct xfs_attr_sf_hdr) +
> > -				XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen,
> > +				xfs_attr_sf_entsize_byname(args->namelen,
> >  						args->valuelen);
> >  
> >  			error = xfs_bmap_add_attrfork(dp, sf_size, rsvd);
> > @@ -523,6 +523,15 @@ xfs_attr_set(
> >   * External routines when attribute list is inside the inode
> >   *========================================================================*/
> >  
> > +static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
> > +
> > +	xfs_attr_shortform_t *sf =
> > +		(xfs_attr_shortform_t *)dp->i_afp->if_u1.if_data;
> > +
> > +	return (be16_to_cpu(sf->hdr.totsize));
> > +
> > +}
> > +
> >  /*
> >   * Add a name to the shortform attribute list structure
> >   * This is the external routine.
> > @@ -555,8 +564,8 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
> >  	    args->valuelen >= XFS_ATTR_SF_ENTSIZE_MAX)
> >  		return -ENOSPC;
> >  
> > -	newsize = XFS_ATTR_SF_TOTSIZE(args->dp);
> > -	newsize += XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
> > +	newsize = xfs_attr_sf_totsize(args->dp);
> > +	newsize += xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
> >  
> >  	forkoff = xfs_attr_shortform_bytesfit(args->dp, newsize);
> >  	if (!forkoff)
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index 7bbc97e0e4d4a..a8a4e21d19726 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -684,9 +684,9 @@ xfs_attr_sf_findname(
> >  	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
> >  	sfe = &sf->list[0];
> >  	end = sf->hdr.count;
> > -	for (i = 0; i < end; sfe = XFS_ATTR_SF_NEXTENTRY(sfe),
> > +	for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
> >  			     base += size, i++) {
> > -		size = XFS_ATTR_SF_ENTSIZE(sfe);
> > +		size = xfs_attr_sf_entsize(sfe);
> >  		if (!xfs_attr_match(args, sfe->namelen, sfe->nameval,
> >  				    sfe->flags))
> >  			continue;
> > @@ -733,7 +733,7 @@ xfs_attr_shortform_add(
> >  		ASSERT(0);
> >  
> >  	offset = (char *)sfe - (char *)sf;
> > -	size = XFS_ATTR_SF_ENTSIZE_BYNAME(args->namelen, args->valuelen);
> > +	size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
> >  	xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
> >  	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> >  	sfe = (xfs_attr_sf_entry_t *)((char *)sf + offset);
> > @@ -792,7 +792,7 @@ xfs_attr_shortform_remove(
> >  	error = xfs_attr_sf_findname(args, &sfe, &base);
> >  	if (error != -EEXIST)
> >  		return error;
> > -	size = XFS_ATTR_SF_ENTSIZE(sfe);
> > +	size = xfs_attr_sf_entsize(sfe);
> >  
> >  	/*
> >  	 * Fix up the attribute fork data, covering the hole
> > @@ -849,7 +849,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
> >  	sf = (xfs_attr_shortform_t *)ifp->if_u1.if_data;
> >  	sfe = &sf->list[0];
> >  	for (i = 0; i < sf->hdr.count;
> > -				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
> > +				sfe = xfs_attr_sf_nextentry(sfe), i++) {
> >  		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
> >  				sfe->flags))
> >  			return -EEXIST;
> > @@ -876,7 +876,7 @@ xfs_attr_shortform_getvalue(
> >  	sf = (xfs_attr_shortform_t *)args->dp->i_afp->if_u1.if_data;
> >  	sfe = &sf->list[0];
> >  	for (i = 0; i < sf->hdr.count;
> > -				sfe = XFS_ATTR_SF_NEXTENTRY(sfe), i++) {
> > +				sfe = xfs_attr_sf_nextentry(sfe), i++) {
> >  		if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
> >  				sfe->flags))
> >  			return xfs_attr_copy_value(args,
> > @@ -951,7 +951,7 @@ xfs_attr_shortform_to_leaf(
> >  		ASSERT(error != -ENOSPC);
> >  		if (error)
> >  			goto out;
> > -		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> > +		sfe = xfs_attr_sf_nextentry(sfe);
> >  	}
> >  	error = 0;
> >  	*leaf_bp = bp;
> > @@ -1049,7 +1049,7 @@ xfs_attr_shortform_verify(
> >  		 * within the data buffer.  The next entry starts after the
> >  		 * name component, so nextentry is an acceptable test.
> >  		 */
> > -		next_sfep = XFS_ATTR_SF_NEXTENTRY(sfep);
> > +		next_sfep = xfs_attr_sf_nextentry(sfep);
> >  		if ((char *)next_sfep > endp)
> >  			return __this_address;
> >  
> > diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
> > index d93012a0be4d0..48906c5196505 100644
> > --- a/fs/xfs/libxfs/xfs_attr_sf.h
> > +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> > @@ -27,16 +27,21 @@ typedef struct xfs_attr_sf_sort {
> >  	unsigned char	*name;		/* name value, pointer into buffer */
> >  } xfs_attr_sf_sort_t;
> >  
> > -#define XFS_ATTR_SF_ENTSIZE_BYNAME(nlen,vlen)	/* space name/value uses */ \
> > -	(((int)sizeof(xfs_attr_sf_entry_t) + (nlen)+(vlen)))
> >  #define XFS_ATTR_SF_ENTSIZE_MAX			/* max space for name&value */ \
> >  	(1 << (NBBY*(int)sizeof(uint8_t)))
> > -#define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
> > -	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)
> > -#define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
> > -	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
> > -#define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
> > -	(be16_to_cpu(((xfs_attr_shortform_t *)	\
> > -		((dp)->i_afp->if_u1.if_data))->hdr.totsize))
> >  
> > +static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> > +	return sizeof(xfs_attr_sf_entry_t) + nlen + vlen;
> > +}
> > +
> > +/* space an entry uses */
> > +static inline int xfs_attr_sf_entsize(xfs_attr_sf_entry_t *sfep) {
> > +	return sizeof(xfs_attr_sf_entry_t) + sfep->namelen + sfep->valuelen;
> > +}
> > +
> > +static inline xfs_attr_sf_entry_t *
> > +xfs_attr_sf_nextentry(xfs_attr_sf_entry_t *sfep) {
> > +	return (xfs_attr_sf_entry_t *)((char *)(sfep) +
> > +				       xfs_attr_sf_entsize(sfep));
> > +}
> >  #endif	/* __XFS_ATTR_SF_H__ */
> > diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> > index 50f922cad91a4..fbe5574f08930 100644
> > --- a/fs/xfs/xfs_attr_list.c
> > +++ b/fs/xfs/xfs_attr_list.c
> > @@ -96,7 +96,7 @@ xfs_attr_shortform_list(
> >  			 */
> >  			if (context->seen_enough)
> >  				break;
> > -			sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> > +			sfe = xfs_attr_sf_nextentry(sfe);
> >  		}
> >  		trace_xfs_attr_list_sf_all(context);
> >  		return 0;
> > @@ -136,7 +136,7 @@ xfs_attr_shortform_list(
> >  		/* These are bytes, and both on-disk, don't endian-flip */
> >  		sbp->valuelen = sfe->valuelen;
> >  		sbp->flags = sfe->flags;
> > -		sfe = XFS_ATTR_SF_NEXTENTRY(sfe);
> > +		sfe = xfs_attr_sf_nextentry(sfe);
> >  		sbp++;
> >  		nsbuf++;
> >  	}
> > -- 
> > 2.26.2
> > 
> 

-- 
Carlos

Re: [PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry

[Carlos Maiolino]

Hi.

> >  #define XFS_ATTR_SF_ENTSIZE(sfep)		/* space an entry uses */ \
> > -	((int)sizeof(xfs_attr_sf_entry_t)-1 + (sfep)->namelen+(sfep)->valuelen)
> > +	((int)sizeof(xfs_attr_sf_entry_t) + (sfep)->namelen+(sfep)->valuelen)
> 
> Can this (and ENTSIZE_BYNAME) use struct_sizeof?

Regarding our talk on #xfs, I've been playing with it a while today, and IMHO,
converting xfs_attr_sf_entsize to use struct_size() is ok, although,
entsize_byname, I don't think it's worth it. entsize_byname doesn't get a
xfs_attr_sf_entry as argument, so it will require to change the callers to
actually pass a struct xfs_attr_sf_entry, and the respective sizes, but, some
callers actually don't even have a xfs_attr_sf_entry to pass into it, so IMHO, I
don't think it's worth to change it, but by any means, I'll send today a V2 of
my patchset containing the changes from V1, and adding the struct_size() into
xfs_attr_sf_entry.

Cheers.

> 
> --D
> 
> >  #define XFS_ATTR_SF_NEXTENTRY(sfep)		/* next entry in struct */ \
> >  	((xfs_attr_sf_entry_t *)((char *)(sfep) + XFS_ATTR_SF_ENTSIZE(sfep)))
> >  #define XFS_ATTR_SF_TOTSIZE(dp)			/* total space in use */ \
> > diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> > index 059ac108b1b39..e86185a1165b3 100644
> > --- a/fs/xfs/libxfs/xfs_da_format.h
> > +++ b/fs/xfs/libxfs/xfs_da_format.h
> > @@ -589,7 +589,7 @@ typedef struct xfs_attr_shortform {
> >  		uint8_t namelen;	/* actual length of name (no NULL) */
> >  		uint8_t valuelen;	/* actual length of value (no NULL) */
> >  		uint8_t flags;	/* flags bits (see xfs_attr_leaf.h) */
> > -		uint8_t nameval[1];	/* name & value bytes concatenated */
> > +		uint8_t nameval[];	/* name & value bytes concatenated */
> >  	} list[1];			/* variable sized array */
> >  } xfs_attr_shortform_t;
> >  
> > -- 
> > 2.26.2
> > 
> 

-- 
Carlos