On Tue, Apr 09, 2024 at 05:56:22PM -0700, Darrick J. Wong wrote:
> From: Allison Henderson <allison.henderson@xxxxxxxxxx>
>
> The parent pointer code needs to do a deferred parent pointer replace
> operation with the xattr log intent code. Declare a new logged xattr
> opcode and push it through the log.
>
> (Formerly titled "xfs: Add new name to attri/d" and described as
> follows:
I don't think this history is very important. The being said,
I suspect this and the previous two patches should be combined into
a single one adding the on-disk formats for parent pointers, and the
commit log could use a complete rewrite saying that it a
> + return false;
> + if (attrp->alfi_old_name_len == 0 ||
> + attrp->alfi_old_name_len > XATTR_NAME_MAX)
> + return false;
> + if (attrp->alfi_new_name_len == 0 ||
> + attrp->alfi_new_name_len > XATTR_NAME_MAX)
> + return false;
Given that we have four copies of this (arguably simple) check,
should we grow a helper for it?
> + if (attrp->alfi_value_len == 0 ||
> + attrp->alfi_value_len > XATTR_SIZE_MAX)
> + return false;
All parent pointer attrs must be sized for exactly the parent_rec,
so we should probably check for that explicitly?
> + if (xfs_attr_log_item_op(old_attrp) == XFS_ATTRI_OP_FLAGS_PPTR_REPLACE) {
Please avoid the overly long line here.
>
> + /* Validate the new attr name */
> + if (new_name_len > 0) {
> + if (item->ri_buf[i].i_len != xlog_calc_iovec_len(new_name_len)) {
.. and here.
And while we're at it, maybe factor the checking for valid xattr
name and value log iovecs into little helper instead of duplicating
them a few times?
