> +/* Verify the consistency of an inline attribute fork. */
> +xfs_failaddr_t
> +xfs_attr_shortform_verify(
> + struct xfs_inode *ip)
> +{
> + struct xfs_attr_shortform *sfp;
> + struct xfs_ifork *ifp;
> + int64_t size;
> +
> + ASSERT(ip->i_af.if_format == XFS_DINODE_FMT_LOCAL);
> + ifp = xfs_ifork_ptr(ip, XFS_ATTR_FORK);
> + sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
> + size = ifp->if_bytes;
> +
> + return xfs_attr_shortform_verify_struct(sfp, size);
Given that xfs_attr_shortform_verify only has a single caller in the
kernel and no extra n in xfsprogs I'd just change the calling
convention to pass the xfs_attr_shortform structure and size there
and not bother with the wrapper.
> +/* Check that an inode's extent does not have invalid flags or bad ranges. */
> +xfs_failaddr_t
> +xfs_bmap_validate_extent(
> + struct xfs_inode *ip,
> + int whichfork,
> + struct xfs_bmbt_irec *irec)
> +{
> + return xfs_bmap_validate_extent_raw(ip->i_mount,
> + XFS_IS_REALTIME_INODE(ip), whichfork, irec);
> +}
.. while this one has a bunch of caller so I expect it's actually
somewhat useful.
> +extern xfs_failaddr_t xfs_dir2_sf_verify_struct(struct xfs_mount *mp,
> + struct xfs_dir2_sf_hdr *sfp, int64_t size);
It would be nice if we didn't add more pointless externs to function
declarations in heders.
> +xfs_failaddr_t
> +xfs_dir2_sf_verify(
> + struct xfs_inode *ip)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
> + struct xfs_dir2_sf_hdr *sfp;
> +
> + ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
> +
> + sfp = (struct xfs_dir2_sf_hdr *)ifp->if_u1.if_data;
> + return xfs_dir2_sf_verify_struct(mp, sfp, ifp->if_bytes);
> +}
This one also only has a single caller in the kernel and user space
combined, so I wou;dn't bother with the wrapper.
> +xfs_failaddr_t
> +xfs_symlink_shortform_verify(
> + struct xfs_inode *ip)
> +{
> + struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
> +
> + ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
> +
> + return xfs_symlink_sf_verify_struct(ifp->if_u1.if_data, ifp->if_bytes);
> +}
Same here.
Once past thes nitpicks the zapping functionality looks fine to me,
but leaves me with a very high level question:
As far as I can tell the inodes with the zapped fork(s) remains in it's
normal place, and normaly accessible, and I think any read will return
zeroes because i_size isn't reset. Which would change the data seen
by an application using it. Don't we need to block access to it until
it is fully repaired?
