Re: [PATCH v4 05/17] xfs: Add xfs_has_attr and subroutines

Allison Collins
2019-11-11T23:38:43Z

On 11/11/19 10:53 AM, Christoph Hellwig wrote:
>> + if (!xfs_inode_hasattr(dp)) {
>> + error = -ENOATTR;
>> + } else if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
>> + ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
>> + error = xfs_attr_shortform_hasname(args, NULL, NULL);
>> + } else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
>> + error = xfs_attr_leaf_hasname(args, &bp);
>> + if (error != -ENOATTR && error != -EEXIST)
>> + goto out;
>> + xfs_trans_brelse(args->trans, bp);
>> + } else {
>> + error = xfs_attr_node_hasname(args, NULL);
>> + }
>> +out:
>> + return error;
>> +}
>
> I think a lot of this would be much simpler without the goto out, e.g.:
>
> if (!xfs_inode_hasattr(dp))
> return -ENOATTR;
>
> if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
> return xfs_attr_shortform_hasname(args, NULL, NULL);
> }
>
> if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> struct xfs_buf *bp;
> int error = xfs_attr_leaf_hasname(args, &bp);
>
> if (error == -ENOATTR || error == -EEXIST)
> xfs_trans_brelse(args->trans, bp);
> return error;
> }
>
> return xfs_attr_node_hasname(args, NULL);
>

Sure, will fix this along with Brians suggestions too. Thanks!
Allison