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

[Allison Collins <allison.henderson@xxxxxxxxxx>]

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