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

[Allison Collins <allison.henderson@oracle.com>]

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