On Mon, Apr 15, 2024 at 10:29:25PM -0700, Christoph Hellwig wrote: > On Mon, Apr 15, 2024 at 06:36:43PM -0700, Darrick J. Wong wrote: > > + if (args->attr_filter & XFS_ATTR_PARENT) > > + xfs_attr_defer_parent(args, > > + XFS_ATTR_DEFER_REMOVE); > > + else > > + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REMOVE); > > > + if (args->attr_filter & XFS_ATTR_PARENT) > > + xfs_attr_defer_parent(args, XFS_ATTR_DEFER_REPLACE); > > + else > > + xfs_attr_defer_add(args, XFS_ATTR_DEFER_REPLACE); > > > + if (args->attr_filter & XFS_ATTR_PARENT) > > + xfs_attr_defer_parent(args, XFS_ATTR_DEFER_SET); > > + else > > + xfs_attr_defer_add(args, XFS_ATTR_DEFER_SET); > > Given how xfs_attr_defer_add/xfs_attr_defer_parent are basically > duplicates except for setting op_flags, shouldn't this move into > xfs_attr_defer_add? I prefer to keep the pptr version separate so that we can assert that the args contents for parent pointers is really correct. Looking at xfs_attr_defer_parent again, it also ought to be checking that args->valuelen == sizeof(xfs_getparents_rec); --D
