* [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t
2020-09-02 14:40 [PATCH 0/4 V2] Clean up xfs_attr_sf_entry Carlos Maiolino
@ 2020-09-02 14:40 ` Carlos Maiolino
2020-09-02 17:35 ` Darrick J. Wong
2020-09-03 8:52 ` Christoph Hellwig
2020-09-02 14:40 ` [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t Carlos Maiolino
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-02 14:40 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:
V2:
- Reordered within the series, no functional changes
fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++--
fs/xfs/libxfs/xfs_attr_sf.h | 11 ++++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 305d4bc073370..76d3814f9dc79 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 bb004fb7944a7..c4afb33079184 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.
@@ -28,15 +27,17 @@ 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(struct xfs_attr_sf_entry)-1 + (nlen)+(vlen)))
#define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
#define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
- ((int)sizeof(xfs_attr_sf_entry_t)-1 + (sfep)->namelen+(sfep)->valuelen)
+ ((int)sizeof(struct xfs_attr_sf_entry)-1 + \
+ (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)))
+ ((struct xfs_attr_sf_entry *)((char *)(sfep) + \
+ XFS_ATTR_SF_ENTSIZE(sfep)))
#define XFS_ATTR_SF_TOTSIZE(dp) /* total space in use */ \
- (be16_to_cpu(((xfs_attr_shortform_t *) \
+ (be16_to_cpu(((struct xfs_attr_shortform *) \
((dp)->i_afp->if_u1.if_data))->hdr.totsize))
#endif /* __XFS_ATTR_SF_H__ */
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t
2020-09-02 14:40 ` [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t Carlos Maiolino
@ 2020-09-02 17:35 ` Darrick J. Wong
2020-09-03 8:52 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-09-02 17:35 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 04:40:56PM +0200, Carlos Maiolino wrote:
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> Changelog:
> V2:
> - Reordered within the series, no functional changes
>
> fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++--
> fs/xfs/libxfs/xfs_attr_sf.h | 11 ++++++-----
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 305d4bc073370..76d3814f9dc79 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 bb004fb7944a7..c4afb33079184 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.
> @@ -28,15 +27,17 @@ 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(struct xfs_attr_sf_entry)-1 + (nlen)+(vlen)))
> #define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
> ((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
> #define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
> - ((int)sizeof(xfs_attr_sf_entry_t)-1 + (sfep)->namelen+(sfep)->valuelen)
> + ((int)sizeof(struct xfs_attr_sf_entry)-1 + \
> + (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)))
> + ((struct xfs_attr_sf_entry *)((char *)(sfep) + \
> + XFS_ATTR_SF_ENTSIZE(sfep)))
> #define XFS_ATTR_SF_TOTSIZE(dp) /* total space in use */ \
> - (be16_to_cpu(((xfs_attr_shortform_t *) \
> + (be16_to_cpu(((struct xfs_attr_shortform *) \
> ((dp)->i_afp->if_u1.if_data))->hdr.totsize))
>
> #endif /* __XFS_ATTR_SF_H__ */
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t
2020-09-02 14:40 ` [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t Carlos Maiolino
2020-09-02 17:35 ` Darrick J. Wong
@ 2020-09-03 8:52 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-09-03 8:52 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t
2020-09-02 14:40 [PATCH 0/4 V2] Clean up xfs_attr_sf_entry Carlos Maiolino
2020-09-02 14:40 ` [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t Carlos Maiolino
@ 2020-09-02 14:40 ` Carlos Maiolino
2020-09-02 17:36 ` Darrick J. Wong
2020-09-03 8:53 ` Christoph Hellwig
2020-09-02 14:40 ` [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry Carlos Maiolino
2020-09-02 14:40 ` [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions Carlos Maiolino
3 siblings, 2 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-02 14:40 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
Changelog:
V2:
- Reordered within the series, no functional changes.
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 ++++++------
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 76d3814f9dc79..d920183b08a99 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 059ac108b1b39..e708b714bf99d 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[1]; /* 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 50f922cad91a4..4eb1d6faecfb2 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t
2020-09-02 14:40 ` [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t Carlos Maiolino
@ 2020-09-02 17:36 ` Darrick J. Wong
2020-09-03 8:28 ` Carlos Maiolino
2020-09-03 8:53 ` Christoph Hellwig
1 sibling, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2020-09-02 17:36 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 04:40:57PM +0200, Carlos Maiolino wrote:
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>
> Changelog:
> V2:
> - Reordered within the series, no functional changes.
>
> 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 ++++++------
> 4 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 76d3814f9dc79..d920183b08a99 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 059ac108b1b39..e708b714bf99d 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[1]; /* 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 50f922cad91a4..4eb1d6faecfb2 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t
2020-09-02 17:36 ` Darrick J. Wong
@ 2020-09-03 8:28 ` Carlos Maiolino
0 siblings, 0 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-03 8:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 10:36:10AM -0700, Darrick J. Wong wrote:
> On Wed, Sep 02, 2020 at 04:40:57PM +0200, Carlos Maiolino wrote:
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >
> > Changelog:
> > V2:
> > - Reordered within the series, no functional changes.
> >
looks like your reply went straight into /dev/null? :)
--
Carlos
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t
2020-09-02 14:40 ` [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t Carlos Maiolino
2020-09-02 17:36 ` Darrick J. Wong
@ 2020-09-03 8:53 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-09-03 8:53 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry
2020-09-02 14:40 [PATCH 0/4 V2] Clean up xfs_attr_sf_entry Carlos Maiolino
2020-09-02 14:40 ` [PATCH V2 1/4] xfs: remove typedef xfs_attr_sf_entry_t Carlos Maiolino
2020-09-02 14:40 ` [PATCH V2 2/4] xfs: Remove typedef xfs_attr_shortform_t Carlos Maiolino
@ 2020-09-02 14:40 ` Carlos Maiolino
2020-09-02 17:52 ` Darrick J. Wong
2020-09-03 9:11 ` Christoph Hellwig
2020-09-02 14:40 ` [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions Carlos Maiolino
3 siblings, 2 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-02 14:40 UTC (permalink / raw)
To: linux-xfs
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>
---
Changelog:
V2:
- Drop wrong change to XFS_ATTR_SF_ENTSIZE_MAX
fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++----
fs/xfs/libxfs/xfs_attr_sf.h | 4 ++--
fs/xfs/libxfs/xfs_da_format.h | 2 +-
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index d920183b08a99..89193871e6a7f 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 c4afb33079184..f608a2966d7f8 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -27,11 +27,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(struct xfs_attr_sf_entry)-1 + (nlen)+(vlen)))
+ (((int)sizeof(struct xfs_attr_sf_entry) + (nlen)+(vlen)))
#define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
#define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
- ((int)sizeof(struct xfs_attr_sf_entry)-1 + \
+ ((int)sizeof(struct xfs_attr_sf_entry) + \
(sfep)->namelen+(sfep)->valuelen)
#define XFS_ATTR_SF_NEXTENTRY(sfep) /* next entry in struct */ \
((struct xfs_attr_sf_entry *)((char *)(sfep) + \
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index e708b714bf99d..b40a4e80f5ee6 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -589,7 +589,7 @@ 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 */
};
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry
2020-09-02 14:40 ` [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry Carlos Maiolino
@ 2020-09-02 17:52 ` Darrick J. Wong
2020-09-03 9:11 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2020-09-02 17:52 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 04:40:58PM +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>
> ---
> Changelog:
>
> V2:
> - Drop wrong change to XFS_ATTR_SF_ENTSIZE_MAX
>
> fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++----
> fs/xfs/libxfs/xfs_attr_sf.h | 4 ++--
> fs/xfs/libxfs/xfs_da_format.h | 2 +-
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index d920183b08a99..89193871e6a7f 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);
Isn't this an open-coded:
bytes += XFS_ATTR_SF_ENTSIZE_BYNAME(name_loc->namelen,
be16_to_cpu(name_loc->valuelen));
Otherwise this patch looks ok to me.
--D
> }
> @@ -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 c4afb33079184..f608a2966d7f8 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -27,11 +27,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(struct xfs_attr_sf_entry)-1 + (nlen)+(vlen)))
> + (((int)sizeof(struct xfs_attr_sf_entry) + (nlen)+(vlen)))
> #define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
> ((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
> #define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
> - ((int)sizeof(struct xfs_attr_sf_entry)-1 + \
> + ((int)sizeof(struct xfs_attr_sf_entry) + \
> (sfep)->namelen+(sfep)->valuelen)
> #define XFS_ATTR_SF_NEXTENTRY(sfep) /* next entry in struct */ \
> ((struct xfs_attr_sf_entry *)((char *)(sfep) + \
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index e708b714bf99d..b40a4e80f5ee6 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -589,7 +589,7 @@ 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 */
> };
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry
2020-09-02 14:40 ` [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry Carlos Maiolino
2020-09-02 17:52 ` Darrick J. Wong
@ 2020-09-03 9:11 ` Christoph Hellwig
1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-09-03 9:11 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 04:40:58PM +0200, Carlos Maiolino wrote:
> nameval is a variable-size array, so, define it as it, and remove all
> the -1 magic number subtractions
Looks good, but two little style nitpicks below.
Reviewed-by: Christoph Hellwig <hch@lst.de>
> 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);
This can be condensed to:
bytes += sizeof(struct xfs_attr_sf_entry) + name_loc->namelen +
be16_to_cpu(name_loc->valuelen);
> index c4afb33079184..f608a2966d7f8 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -27,11 +27,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(struct xfs_attr_sf_entry)-1 + (nlen)+(vlen)))
> + (((int)sizeof(struct xfs_attr_sf_entry) + (nlen)+(vlen)))
We can drop the int cast, but more importantly please add whitespaces
before and after the + operator.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-02 14:40 [PATCH 0/4 V2] Clean up xfs_attr_sf_entry Carlos Maiolino
` (2 preceding siblings ...)
2020-09-02 14:40 ` [PATCH V2 3/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry Carlos Maiolino
@ 2020-09-02 14:40 ` Carlos Maiolino
2020-09-02 17:54 ` Darrick J. Wong
2020-09-03 9:14 ` Christoph Hellwig
3 siblings, 2 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-02 14:40 UTC (permalink / raw)
To: linux-xfs
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>
---
V2:
- keep macro comments above inline functions
- Use struct_size() on xfs_attr_sf_entsize()
fs/xfs/libxfs/xfs_attr.c | 13 ++++++++++---
fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
fs/xfs/libxfs/xfs_attr_sf.h | 28 +++++++++++++++++-----------
fs/xfs/xfs_attr_list.c | 4 ++--
4 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 2e055c079f397..ff1fa2ed40ab9 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,13 @@ xfs_attr_set(
* External routines when attribute list is inside the inode
*========================================================================*/
+/* total space in use */
+static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
+ struct xfs_attr_shortform *sf =
+ (struct xfs_attr_shortform *)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 +562,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 89193871e6a7f..f64ab351b760c 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 = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
sfe = (struct xfs_attr_sf_entry *)((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 = (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++) {
+ 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 = (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++) {
+ 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 f608a2966d7f8..0d761306da965 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -26,18 +26,24 @@ 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(struct xfs_attr_sf_entry) + (nlen)+(vlen)))
#define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
-#define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
- ((int)sizeof(struct xfs_attr_sf_entry) + \
- (sfep)->namelen+(sfep)->valuelen)
-#define XFS_ATTR_SF_NEXTENTRY(sfep) /* next entry in struct */ \
- ((struct xfs_attr_sf_entry *)((char *)(sfep) + \
- XFS_ATTR_SF_ENTSIZE(sfep)))
-#define XFS_ATTR_SF_TOTSIZE(dp) /* total space in use */ \
- (be16_to_cpu(((struct xfs_attr_shortform *) \
- ((dp)->i_afp->if_u1.if_data))->hdr.totsize))
+
+/* space name/value uses */
+static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
+ return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
+}
+
+/* space an entry uses */
+static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
+ return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
+}
+
+/* next entry in struct */
+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__ */
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 4eb1d6faecfb2..8f8837fe21cf0 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-02 14:40 ` [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions Carlos Maiolino
@ 2020-09-02 17:54 ` Darrick J. Wong
2020-09-03 10:36 ` Carlos Maiolino
2020-09-03 9:14 ` Christoph Hellwig
1 sibling, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2020-09-02 17:54 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
On Wed, Sep 02, 2020 at 04:40:59PM +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>
> ---
>
> V2:
> - keep macro comments above inline functions
> - Use struct_size() on xfs_attr_sf_entsize()
>
> fs/xfs/libxfs/xfs_attr.c | 13 ++++++++++---
> fs/xfs/libxfs/xfs_attr_leaf.c | 16 ++++++++--------
> fs/xfs/libxfs/xfs_attr_sf.h | 28 +++++++++++++++++-----------
> fs/xfs/xfs_attr_list.c | 4 ++--
> 4 files changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 2e055c079f397..ff1fa2ed40ab9 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,13 @@ xfs_attr_set(
> * External routines when attribute list is inside the inode
> *========================================================================*/
>
> +/* total space in use */
> +static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
> + struct xfs_attr_shortform *sf =
> + (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
> + return be16_to_cpu(sf->hdr.totsize);
Please add a newline between the variable declaration and the return
statement.
> +}
> +
> /*
> * Add a name to the shortform attribute list structure
> * This is the external routine.
> @@ -555,8 +562,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 89193871e6a7f..f64ab351b760c 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 = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
> sfe = (struct xfs_attr_sf_entry *)((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 = (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++) {
> + 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 = (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++) {
> + 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 f608a2966d7f8..0d761306da965 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -26,18 +26,24 @@ 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(struct xfs_attr_sf_entry) + (nlen)+(vlen)))
> #define XFS_ATTR_SF_ENTSIZE_MAX /* max space for name&value */ \
> ((1 << (NBBY*(int)sizeof(uint8_t))) - 1)
> -#define XFS_ATTR_SF_ENTSIZE(sfep) /* space an entry uses */ \
> - ((int)sizeof(struct xfs_attr_sf_entry) + \
> - (sfep)->namelen+(sfep)->valuelen)
> -#define XFS_ATTR_SF_NEXTENTRY(sfep) /* next entry in struct */ \
> - ((struct xfs_attr_sf_entry *)((char *)(sfep) + \
> - XFS_ATTR_SF_ENTSIZE(sfep)))
> -#define XFS_ATTR_SF_TOTSIZE(dp) /* total space in use */ \
> - (be16_to_cpu(((struct xfs_attr_shortform *) \
> - ((dp)->i_afp->if_u1.if_data))->hdr.totsize))
> +
> +/* space name/value uses */
> +static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> + return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
/me suspects you could (ab)use struct_size here too, e.g.
return struct_size((struct xfs_attr_sf_entry *)NULL, nameval,
nlen + vlen);
Though now I look at the casting mess and think NAH.
Ok never mind. :)
The patch looks ok, modulo that spacing thing from above.
--D
> +}
> +
> +/* space an entry uses */
> +static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
> + return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
> +}
> +
> +/* next entry in struct */
> +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__ */
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 4eb1d6faecfb2..8f8837fe21cf0 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-02 17:54 ` Darrick J. Wong
@ 2020-09-03 10:36 ` Carlos Maiolino
0 siblings, 0 replies; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-03 10:36 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
>
> /me suspects you could (ab)use struct_size here too, e.g.
>
> return struct_size((struct xfs_attr_sf_entry *)NULL, nameval,
> nlen + vlen);
>
> Though now I look at the casting mess and think NAH.
> Ok never mind. :)
I think this could work, yes, but it just makes the inline function hard to read
with no real gain IMHO.
>
> The patch looks ok, modulo that spacing thing from above.
Fixing. Thanks for the review :)
>
> --D
>
> > +}
> > +
> > +/* space an entry uses */
> > +static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
> > + return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
> > +}
> > +
> > +/* next entry in struct */
> > +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__ */
> > diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> > index 4eb1d6faecfb2..8f8837fe21cf0 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-02 14:40 ` [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions Carlos Maiolino
2020-09-02 17:54 ` Darrick J. Wong
@ 2020-09-03 9:14 ` Christoph Hellwig
2020-09-03 10:38 ` Carlos Maiolino
1 sibling, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2020-09-03 9:14 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: linux-xfs
> +/* total space in use */
> +static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
> + struct xfs_attr_shortform *sf =
> + (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
> + return be16_to_cpu(sf->hdr.totsize);
> +}
The opening curly brace should go on a line of its own.
> +/* space name/value uses */
> +static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> + return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
> +}
> +
> +/* space an entry uses */
> +static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
> + return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
> +}
> +
> +/* next entry in struct */
> +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));
> +}
Same for these. Also if you cast to void * instead of char * in
xfs_attr_sf_nextentry (and gcc extension we make heavy use of), you
don't need the case back.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-03 9:14 ` Christoph Hellwig
@ 2020-09-03 10:38 ` Carlos Maiolino
2020-09-03 10:39 ` Christoph Hellwig
0 siblings, 1 reply; 17+ messages in thread
From: Carlos Maiolino @ 2020-09-03 10:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Sep 03, 2020 at 10:14:36AM +0100, Christoph Hellwig wrote:
> > +/* total space in use */
> > +static inline int xfs_attr_sf_totsize(struct xfs_inode *dp) {
> > + struct xfs_attr_shortform *sf =
> > + (struct xfs_attr_shortform *)dp->i_afp->if_u1.if_data;
> > + return be16_to_cpu(sf->hdr.totsize);
> > +}
>
> The opening curly brace should go on a line of its own.
>
Thanks for spotting this, I'll fix on the next version
> > +/* space name/value uses */
> > +static inline int xfs_attr_sf_entsize_byname(uint8_t nlen, uint8_t vlen) {
> > + return sizeof(struct xfs_attr_sf_entry) + nlen + vlen;
> > +}
> > +
> > +/* space an entry uses */
> > +static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep) {
> > + return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
> > +}
> > +
> > +/* next entry in struct */
> > +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));
> > +}
>
> Same for these. Also if you cast to void * instead of char * in
> xfs_attr_sf_nextentry (and gcc extension we make heavy use of), you
> don't need the case back.
I believe you meant cast here? For sure, looks a good simplification, I'll add
it. Thanks again!
--
Carlos
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH V2 4/4] xfs: Convert xfs_attr_sf macros to inline functions
2020-09-03 10:38 ` Carlos Maiolino
@ 2020-09-03 10:39 ` Christoph Hellwig
0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2020-09-03 10:39 UTC (permalink / raw)
To: Christoph Hellwig, linux-xfs
On Thu, Sep 03, 2020 at 12:38:21PM +0200, Carlos Maiolino wrote:
> > Same for these. Also if you cast to void * instead of char * in
> > xfs_attr_sf_nextentry (and gcc extension we make heavy use of), you
> > don't need the case back.
>
> I believe you meant cast here? For sure, looks a good simplification, I'll add
> it. Thanks again!
Yes, sorry.
^ permalink raw reply [flat|nested] 17+ messages in thread