Re: [PATCH v5 08/14] xfs: Factor up xfs_attr_try_sf_addname

Christoph Hellwig
2019-12-24T12:25:55Z

On Wed, Dec 11, 2019 at 09:15:07PM -0700, Allison Collins wrote:
> New delayed attribute routines cannot handle transactions. We
> can factor up the commit, but there is little left in this
> function other than some error handling and an ichgtime. So
> hoist all of xfs_attr_try_sf_addname up at this time. We will
> remove all the commits in this set.

I really don't like this one. xfs_attr_try_sf_addname is a nice
abstration, so merging it into the caller makes the code much harder
to understand. If that is different after changes to the transaction
change it can be removed at that point, but merging all the different
attr formats into one big monster function is a bad idea.

Also Factor up still sounds very, very strange to me. I would
have worded it as "merge xfs_attr_try_sf_addname into its only caller"