On Wed, Aug 18, 2021 at 10:56:08AM +1000, Dave Chinner wrote: > On Thu, Aug 12, 2021 at 09:03:34AM +0100, Christoph Hellwig wrote: > > On Tue, Aug 10, 2021 at 03:24:37PM +1000, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > xfs_has_attr() is poorly named. It has global scope as it is defined > > > in a header file, but it has no namespace scope that tells us what > > > it is checking has attributes. It's not even clear what "has_attr" > > > means, because what it is actually doing is an attribute fork lookup > > > to see if the attribute exists. > > > > > > Upcoming patches use this "xfs_has_<foo>" namespace for global > > > filesystem features, which conflicts with this function. > > > > > > Rename xfs_has_attr() to xfs_attr_lookup() and make it a static > > > function, freeing up the "xfs_has_" namespace for global scope > > > usage. > > > > Why not kill this function entirely as I suggested last time? > > Because I think it's the wrong cleanup to apply to xfs_attr_set(). > xfs_attr_set() needs to be split into two - a set() function and a > remove() function to get rid of all the conditional "if > (arg->value)" logic in it that separates set from remove. Most > of the code in the function is under such if/else clauses, and the > set() code is much more complex than the remove() case. Folding the > attr lookup into the xfs_attr_set() doesn't do anything to address > this high level badness, and to split it appropriately we need to > keep the common attr lookup code in it's own function. > > I updated the patch to has a single xfs_attr_lookup() call instead > of one per branch, but I don't think removing the helper is the > right way to go here... I prefer we leave the xattr code alone (renaming things to avoid name conflicts is ok) and concentrated on helping Allison land xattr logging. Once that's done, xfs_attr_set and xfs_attr_remove become trivial frontends to a deferred operation. --D > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
