On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Convert the last of the open coded corruption check and report idioms to
> use the XFS_IS_CORRUPT macro.
hmmm.
> + if (XFS_IS_CORRUPT(mp,
> + ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
This pattern is weird. It looks like there are two separate logic
statements to the if() condition, when in fact the second line is
part of the XFS_IS_CORRUPT() macro.
It just looks wrong to me, especially when everything other
multi-line macro is indented based on the indenting of the macro
parameters....
Yes, in this case it looks a bit strange, too:
if (XFS_IS_CORRUPT(mp,
ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
but there is no mistaking it for separate logic statements.
I kinda value being able to glance at the indent levels to see
separate logic elements....
> - if (unlikely(
> - be32_to_cpu(sib_info->back) != last_blkno ||
> - sib_info->magic != dead_info->magic)) {
> - XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
> - XFS_ERRLEVEL_LOW, mp);
> + if (XFS_IS_CORRUPT(mp,
> + be32_to_cpu(sib_info->back) != last_blkno ||
> + sib_info->magic != dead_info->magic)) {
> error = -EFSCORRUPTED;
> goto done;
> }
This is kind of what I mean - is it two or three logic statments
here? No, it's actually one, but it has two nested checks...
There's a few other list this that are somewhat non-obvious as to
the logic...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
