Re: [PATCH v7 10/11] xfs: make the calculation generic in xfs_sb_validate_fsb_count()

Pankaj Raghav (Samsung)
2024-06-17T16:09:52Z

On Thu, Jun 13, 2024 at 10:45:45AM +0200, Christoph Hellwig wrote:
> > + uint64_t max_index;
> > + uint64_t max_bytes;
> > +
> > ASSERT(PAGE_SHIFT >= sbp->sb_blocklog);
> > ASSERT(sbp->sb_blocklog >= BBSHIFT);
> >
> > + if (check_shl_overflow(nblocks, sbp->sb_blocklog, &max_bytes))
> > + return -EFBIG;
> > +
> > /* Limited by ULONG_MAX of page cache index */
> > - if (nblocks >> (PAGE_SHIFT - sbp->sb_blocklog) > ULONG_MAX)
> > + max_index = max_bytes >> PAGE_SHIFT;
> > +
> > + if (max_index > ULONG_MAX)
>
> Do we really need the max_index variable for a single user here?
> Or do you plan to add more uses of it later (can't really think of one
> though)?

Yeah, we could just inline it:

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index a99454208807..c6933440f806 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -132,7 +132,6 @@ xfs_sb_validate_fsb_count(
xfs_sb_t *sbp,
uint64_t nblocks)
{
- uint64_t max_index;
uint64_t max_bytes;

ASSERT(sbp->sb_blocklog >= BBSHIFT);
@@ -141,9 +140,7 @@ xfs_sb_validate_fsb_count(
return -EFBIG;

/* Limited by ULONG_MAX of page cache index */
- max_index = max_bytes >> PAGE_SHIFT;
-
- if (max_index > ULONG_MAX)
+ if (max_bytes >> PAGE_SHIFT > ULONG_MAX)
return -EFBIG;
return 0;
}

>