This patch demonstrates very well why I think having these magic defines and the comments in a header makes no sense. > +/* > + * Remapping an extent involves unmapping the existing extent and mapping in the > + * new extent. > + * > + * When unmapping, an extent containing the entire unmap range can be split into > + * two extents, > + * i.e. | Old extent | hole | Old extent | > + * Hence extent count increases by 1. > + * > + * Mapping in the new extent into the destination file can increase the extent > + * count by 1. > + */ > +#define XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written) \ > + (((smap_real) ? 1 : 0) + ((dmap_written) ? 1 : 0)) > + > /* > * Fork handling. > */ > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index 4f0198f636ad..c9f9ff68b5bb 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -1099,6 +1099,11 @@ xfs_reflink_remap_extent( > goto out_cancel; > } > > + error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK, > + XFS_IEXT_REFLINK_REMAP_CNT(smap_real, dmap_written)); > + if (error) > + goto out_cancel; > + This is a completely mess. If OTOH xfs_reflink_remap_extent had a local variable for the potential max number of extents, which is incremented near the initialization of smap_real and dmap_written, with a nice comment near to each increment it would make complete sense to the reader.
