From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Carlos Maiolino <cmaiolino@redhat.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/4] xfs: Convert xfs_attr_sf macros to inline functions
Date: Mon, 31 Aug 2020 08:34:05 -0700 [thread overview]
Message-ID: <20200831153405.GB6096@magnolia> (raw)
In-Reply-To: <20200831130423.136509-3-cmaiolino@redhat.com>
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
>
next prev parent reply other threads:[~2020-08-31 15:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-31 13:04 [PATCH 0/4] Clean up xfs_attr_sf_entry Carlos Maiolino
2020-08-31 13:04 ` [PATCH 1/4] xfs: Use variable-size array for nameval in xfs_attr_sf_entry Carlos Maiolino
2020-08-31 14:53 ` Eric Sandeen
2020-08-31 14:54 ` Eric Sandeen
2020-08-31 15:31 ` Darrick J. Wong
2020-09-02 11:13 ` Carlos Maiolino
2020-08-31 13:04 ` [PATCH 2/4] xfs: Convert xfs_attr_sf macros to inline functions Carlos Maiolino
2020-08-31 15:34 ` Darrick J. Wong [this message]
2020-09-01 7:43 ` Carlos Maiolino
2020-08-31 13:04 ` [PATCH 3/4] xfs: remove typedef xfs_attr_sf_entry_t Carlos Maiolino
2020-08-31 15:34 ` Darrick J. Wong
2020-08-31 13:04 ` [PATCH 4/4] xfs: Remove typedef xfs_attr_shortform_t Carlos Maiolino
2020-08-31 15:35 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200831153405.GB6096@magnolia \
--to=darrick.wong@oracle.com \
--cc=cmaiolino@redhat.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).