Skip to content

Commit 6c8d169

Browse files
Christoph HellwigChandan Babu R
authored andcommitted
xfs: simplify xfs_attr_sf_findname
xfs_attr_sf_findname has the simple job of finding a xfs_attr_sf_entry in the attr fork, but the convoluted calling convention obfuscates that. Return the found entry as the return value instead of an pointer argument, as the -ENOATTR/-EEXIST can be trivally derived from that, and remove the basep argument, as it is equivalent of the offset of sfe in the data for if an sfe was found, or an offset of totsize if not was found. To simplify the totsize computation add a xfs_attr_sf_endptr helper that returns the imaginative xfs_attr_sf_entry at the end of the current attrs. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
1 parent 14f2e4a commit 6c8d169

File tree

4 files changed

+48
-66
lines changed

4 files changed

+48
-66
lines changed

fs/xfs/libxfs/xfs_attr.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,8 +862,11 @@ xfs_attr_lookup(
862862
if (!xfs_inode_hasattr(dp))
863863
return -ENOATTR;
864864

865-
if (dp->i_af.if_format == XFS_DINODE_FMT_LOCAL)
866-
return xfs_attr_sf_findname(args, NULL, NULL);
865+
if (dp->i_af.if_format == XFS_DINODE_FMT_LOCAL) {
866+
if (xfs_attr_sf_findname(args))
867+
return -EEXIST;
868+
return -ENOATTR;
869+
}
867870

868871
if (xfs_attr_is_leaf(dp)) {
869872
error = xfs_attr_leaf_hasname(args, &bp);

fs/xfs/libxfs/xfs_attr_leaf.c

Lines changed: 35 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -698,47 +698,24 @@ xfs_attr_shortform_create(
698698
}
699699

700700
/*
701-
* Return -EEXIST if attr is found, or -ENOATTR if not
702-
* args: args containing attribute name and namelen
703-
* sfep: If not null, pointer will be set to the last attr entry found on
704-
-EEXIST. On -ENOATTR pointer is left at the last entry in the list
705-
* basep: If not null, pointer is set to the byte offset of the entry in the
706-
* list on -EEXIST. On -ENOATTR, pointer is left at the byte offset of
707-
* the last entry in the list
701+
* Return the entry if the attr in args is found, or NULL if not.
708702
*/
709-
int
703+
struct xfs_attr_sf_entry *
710704
xfs_attr_sf_findname(
711-
struct xfs_da_args *args,
712-
struct xfs_attr_sf_entry **sfep,
713-
unsigned int *basep)
705+
struct xfs_da_args *args)
714706
{
715-
struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
716-
struct xfs_attr_sf_entry *sfe;
717-
unsigned int base = sizeof(struct xfs_attr_sf_hdr);
718-
int size = 0;
719-
int end;
720-
int i;
707+
struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
708+
struct xfs_attr_sf_entry *sfe;
721709

722-
sfe = &sf->list[0];
723-
end = sf->hdr.count;
724-
for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
725-
base += size, i++) {
726-
size = xfs_attr_sf_entsize(sfe);
727-
if (!xfs_attr_match(args, sfe->namelen, sfe->nameval,
728-
sfe->flags))
729-
continue;
730-
break;
710+
for (sfe = &sf->list[0];
711+
sfe < xfs_attr_sf_endptr(sf);
712+
sfe = xfs_attr_sf_nextentry(sfe)) {
713+
if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
714+
sfe->flags))
715+
return sfe;
731716
}
732717

733-
if (sfep != NULL)
734-
*sfep = sfe;
735-
736-
if (basep != NULL)
737-
*basep = base;
738-
739-
if (i == end)
740-
return -ENOATTR;
741-
return -EEXIST;
718+
return NULL;
742719
}
743720

744721
/*
@@ -755,21 +732,19 @@ xfs_attr_shortform_add(
755732
struct xfs_ifork *ifp = &dp->i_af;
756733
struct xfs_attr_shortform *sf = ifp->if_data;
757734
struct xfs_attr_sf_entry *sfe;
758-
int offset, size;
735+
int size;
759736

760737
trace_xfs_attr_sf_add(args);
761738

762739
dp->i_forkoff = forkoff;
763740

764741
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
765-
if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
766-
ASSERT(0);
742+
ASSERT(!xfs_attr_sf_findname(args));
767743

768-
offset = (char *)sfe - (char *)sf;
769744
size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
770745
sf = xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
771-
sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
772746

747+
sfe = xfs_attr_sf_endptr(sf);
773748
sfe->namelen = args->namelen;
774749
sfe->valuelen = args->valuelen;
775750
sfe->flags = args->attr_filter;
@@ -809,39 +784,38 @@ xfs_attr_sf_removename(
809784
struct xfs_mount *mp = dp->i_mount;
810785
struct xfs_attr_shortform *sf = dp->i_af.if_data;
811786
struct xfs_attr_sf_entry *sfe;
812-
int size = 0, end, totsize;
813-
unsigned int base;
814-
int error;
787+
uint16_t totsize = be16_to_cpu(sf->hdr.totsize);
788+
void *next, *end;
789+
int size = 0;
815790

816791
trace_xfs_attr_sf_remove(args);
817792

818-
error = xfs_attr_sf_findname(args, &sfe, &base);
819-
820-
/*
821-
* If we are recovering an operation, finding nothing to
822-
* remove is not an error - it just means there was nothing
823-
* to clean up.
824-
*/
825-
if (error == -ENOATTR && (args->op_flags & XFS_DA_OP_RECOVERY))
826-
return 0;
827-
if (error != -EEXIST)
828-
return error;
829-
size = xfs_attr_sf_entsize(sfe);
793+
sfe = xfs_attr_sf_findname(args);
794+
if (!sfe) {
795+
/*
796+
* If we are recovering an operation, finding nothing to remove
797+
* is not an error, it just means there was nothing to clean up.
798+
*/
799+
if (args->op_flags & XFS_DA_OP_RECOVERY)
800+
return 0;
801+
return -ENOATTR;
802+
}
830803

831804
/*
832805
* Fix up the attribute fork data, covering the hole
833806
*/
834-
end = base + size;
835-
totsize = be16_to_cpu(sf->hdr.totsize);
836-
if (end != totsize)
837-
memmove(&((char *)sf)[base], &((char *)sf)[end], totsize - end);
807+
size = xfs_attr_sf_entsize(sfe);
808+
next = xfs_attr_sf_nextentry(sfe);
809+
end = xfs_attr_sf_endptr(sf);
810+
if (next < end)
811+
memmove(sfe, next, end - next);
838812
sf->hdr.count--;
839-
be16_add_cpu(&sf->hdr.totsize, -size);
813+
totsize -= size;
814+
sf->hdr.totsize = cpu_to_be16(totsize);
840815

841816
/*
842817
* Fix up the start offset of the attribute fork
843818
*/
844-
totsize -= size;
845819
if (totsize == sizeof(xfs_attr_sf_hdr_t) && xfs_has_attr2(mp) &&
846820
(dp->i_df.if_format != XFS_DINODE_FMT_BTREE) &&
847821
!(args->op_flags & (XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE))) {

fs/xfs/libxfs/xfs_attr_leaf.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,7 @@ int xfs_attr_shortform_lookup(struct xfs_da_args *args);
5151
int xfs_attr_shortform_getvalue(struct xfs_da_args *args);
5252
int xfs_attr_shortform_to_leaf(struct xfs_da_args *args);
5353
int xfs_attr_sf_removename(struct xfs_da_args *args);
54-
int xfs_attr_sf_findname(struct xfs_da_args *args,
55-
struct xfs_attr_sf_entry **sfep,
56-
unsigned int *basep);
54+
struct xfs_attr_sf_entry *xfs_attr_sf_findname(struct xfs_da_args *args);
5755
int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
5856
int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
5957
xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_shortform *sfp,

fs/xfs/libxfs/xfs_attr_sf.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,11 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
4848
return (void *)sfep + xfs_attr_sf_entsize(sfep);
4949
}
5050

51+
/* pointer to the space after the last entry, e.g. for adding a new one */
52+
static inline struct xfs_attr_sf_entry *
53+
xfs_attr_sf_endptr(struct xfs_attr_shortform *sf)
54+
{
55+
return (void *)sf + be16_to_cpu(sf->hdr.totsize);
56+
}
57+
5158
#endif /* __XFS_ATTR_SF_H__ */

0 commit comments

Comments
 (0)