* [PATCH -next 0/2] Simplify if-else condition for clean code @ 2022-08-30 13:39 Zeng Heng 2022-08-30 13:39 ` [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign Zeng Heng 2022-08-30 13:39 ` [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared Zeng Heng 0 siblings, 2 replies; 5+ messages in thread From: Zeng Heng @ 2022-08-30 13:39 UTC (permalink / raw) To: djwong; +Cc: linux-xfs, linux-kernel, zengheng4 Simplify if-else condition for clean code, which makes them easier to understand. Zeng Heng (2): xfs: simplify if-else condition in xfs_validate_new_dalign xfs: simplify if-else condition in xfs_reflink_trim_around_shared fs/xfs/xfs_mount.c | 38 ++++++++++++++++++++------------------ fs/xfs/xfs_reflink.c | 22 ++++++++++++---------- 2 files changed, 32 insertions(+), 28 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign 2022-08-30 13:39 [PATCH -next 0/2] Simplify if-else condition for clean code Zeng Heng @ 2022-08-30 13:39 ` Zeng Heng 2022-08-30 15:19 ` Darrick J. Wong 2022-08-30 13:39 ` [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared Zeng Heng 1 sibling, 1 reply; 5+ messages in thread From: Zeng Heng @ 2022-08-30 13:39 UTC (permalink / raw) To: djwong; +Cc: linux-xfs, linux-kernel, zengheng4 "else" is not generally useful after a return, so remove them which makes if condition a bit more clear. There is no logical changes. Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- fs/xfs/xfs_mount.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index f10c88cee116..e8bb3c2e847e 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -300,26 +300,28 @@ xfs_validate_new_dalign( "alignment check failed: sunit/swidth vs. blocksize(%d)", mp->m_sb.sb_blocksize); return -EINVAL; - } else { - /* - * Convert the stripe unit and width to FSBs. - */ - mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign); - if (mp->m_dalign && (mp->m_sb.sb_agblocks % mp->m_dalign)) { - xfs_warn(mp, - "alignment check failed: sunit/swidth vs. agsize(%d)", - mp->m_sb.sb_agblocks); - return -EINVAL; - } else if (mp->m_dalign) { - mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth); - } else { - xfs_warn(mp, - "alignment check failed: sunit(%d) less than bsize(%d)", - mp->m_dalign, mp->m_sb.sb_blocksize); - return -EINVAL; - } } + /* + * Convert the stripe unit and width to FSBs. + */ + mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign); + if (mp->m_dalign && (mp->m_sb.sb_agblocks % mp->m_dalign)) { + xfs_warn(mp, + "alignment check failed: sunit/swidth vs. agsize(%d)", + mp->m_sb.sb_agblocks); + return -EINVAL; + } + + if (!mp->m_dalign) { + xfs_warn(mp, + "alignment check failed: sunit(%d) less than bsize(%d)", + mp->m_dalign, mp->m_sb.sb_blocksize); + return -EINVAL; + } + + mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth); + if (!xfs_has_dalign(mp)) { xfs_warn(mp, "cannot change alignment: superblock does not support data alignment"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign 2022-08-30 13:39 ` [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign Zeng Heng @ 2022-08-30 15:19 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2022-08-30 15:19 UTC (permalink / raw) To: Zeng Heng; +Cc: linux-xfs, linux-kernel On Tue, Aug 30, 2022 at 09:39:38PM +0800, Zeng Heng wrote: > "else" is not generally useful after a return, > so remove them which makes if condition a bit > more clear. > > There is no logical changes. > > Signed-off-by: Zeng Heng <zengheng4@huawei.com> Yep. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/xfs_mount.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index f10c88cee116..e8bb3c2e847e 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -300,26 +300,28 @@ xfs_validate_new_dalign( > "alignment check failed: sunit/swidth vs. blocksize(%d)", > mp->m_sb.sb_blocksize); > return -EINVAL; > - } else { > - /* > - * Convert the stripe unit and width to FSBs. > - */ > - mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign); > - if (mp->m_dalign && (mp->m_sb.sb_agblocks % mp->m_dalign)) { > - xfs_warn(mp, > - "alignment check failed: sunit/swidth vs. agsize(%d)", > - mp->m_sb.sb_agblocks); > - return -EINVAL; > - } else if (mp->m_dalign) { > - mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth); > - } else { > - xfs_warn(mp, > - "alignment check failed: sunit(%d) less than bsize(%d)", > - mp->m_dalign, mp->m_sb.sb_blocksize); > - return -EINVAL; > - } > } > > + /* > + * Convert the stripe unit and width to FSBs. > + */ > + mp->m_dalign = XFS_BB_TO_FSBT(mp, mp->m_dalign); > + if (mp->m_dalign && (mp->m_sb.sb_agblocks % mp->m_dalign)) { > + xfs_warn(mp, > + "alignment check failed: sunit/swidth vs. agsize(%d)", > + mp->m_sb.sb_agblocks); > + return -EINVAL; > + } > + > + if (!mp->m_dalign) { > + xfs_warn(mp, > + "alignment check failed: sunit(%d) less than bsize(%d)", > + mp->m_dalign, mp->m_sb.sb_blocksize); > + return -EINVAL; > + } > + > + mp->m_swidth = XFS_BB_TO_FSBT(mp, mp->m_swidth); I think this reorganization of the if test logic is correct, but I really wish this unit abuse ("m_swidth is a BB for short periods of time and fsblock everywhere else") would get fixed to simplify analysis and prevent us from stumbling over things like that some time later. So for this change, Reviewed-by: Darrick J. Wong <djwong@kernel.org> But I would be very happy to see some tripping hazard removal. ;) --D > + > if (!xfs_has_dalign(mp)) { > xfs_warn(mp, > "cannot change alignment: superblock does not support data alignment"); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared 2022-08-30 13:39 [PATCH -next 0/2] Simplify if-else condition for clean code Zeng Heng 2022-08-30 13:39 ` [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign Zeng Heng @ 2022-08-30 13:39 ` Zeng Heng 2022-08-30 15:12 ` Darrick J. Wong 1 sibling, 1 reply; 5+ messages in thread From: Zeng Heng @ 2022-08-30 13:39 UTC (permalink / raw) To: djwong; +Cc: linux-xfs, linux-kernel, zengheng4 "else" is not generally useful after a return, so remove it for clean code. There is no logical changes. Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- fs/xfs/xfs_reflink.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index 251f20ddd368..93bdd25680bc 100644 --- a/fs/xfs/xfs_reflink.c +++ b/fs/xfs/xfs_reflink.c @@ -200,7 +200,9 @@ xfs_reflink_trim_around_shared( if (fbno == NULLAGBLOCK) { /* No shared blocks at all. */ return 0; - } else if (fbno == agbno) { + } + + if (fbno == agbno) { /* * The start of this extent is shared. Truncate the * mapping at the end of the shared region so that a @@ -210,16 +212,16 @@ xfs_reflink_trim_around_shared( irec->br_blockcount = flen; *shared = true; return 0; - } else { - /* - * There's a shared extent midway through this extent. - * Truncate the mapping at the start of the shared - * extent so that a subsequent iteration starts at the - * start of the shared region. - */ - irec->br_blockcount = fbno - agbno; - return 0; } + + /* + * There's a shared extent midway through this extent. + * Truncate the mapping at the start of the shared + * extent so that a subsequent iteration starts at the + * start of the shared region. + */ + irec->br_blockcount = fbno - agbno; + return 0; } int -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared 2022-08-30 13:39 ` [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared Zeng Heng @ 2022-08-30 15:12 ` Darrick J. Wong 0 siblings, 0 replies; 5+ messages in thread From: Darrick J. Wong @ 2022-08-30 15:12 UTC (permalink / raw) To: Zeng Heng; +Cc: linux-xfs, linux-kernel On Tue, Aug 30, 2022 at 09:39:39PM +0800, Zeng Heng wrote: > "else" is not generally useful after a return, > so remove it for clean code. > > There is no logical changes. > > Signed-off-by: Zeng Heng <zengheng4@huawei.com> Looks correct to me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/xfs_reflink.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index 251f20ddd368..93bdd25680bc 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -200,7 +200,9 @@ xfs_reflink_trim_around_shared( > if (fbno == NULLAGBLOCK) { > /* No shared blocks at all. */ > return 0; > - } else if (fbno == agbno) { > + } > + > + if (fbno == agbno) { > /* > * The start of this extent is shared. Truncate the > * mapping at the end of the shared region so that a > @@ -210,16 +212,16 @@ xfs_reflink_trim_around_shared( > irec->br_blockcount = flen; > *shared = true; > return 0; > - } else { > - /* > - * There's a shared extent midway through this extent. > - * Truncate the mapping at the start of the shared > - * extent so that a subsequent iteration starts at the > - * start of the shared region. > - */ > - irec->br_blockcount = fbno - agbno; > - return 0; > } > + > + /* > + * There's a shared extent midway through this extent. > + * Truncate the mapping at the start of the shared > + * extent so that a subsequent iteration starts at the > + * start of the shared region. > + */ > + irec->br_blockcount = fbno - agbno; > + return 0; > } > > int > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-30 15:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-30 13:39 [PATCH -next 0/2] Simplify if-else condition for clean code Zeng Heng 2022-08-30 13:39 ` [PATCH -next 1/2] xfs: simplify if-else condition in xfs_validate_new_dalign Zeng Heng 2022-08-30 15:19 ` Darrick J. Wong 2022-08-30 13:39 ` [PATCH -next 2/2] xfs: simplify if-else condition in xfs_reflink_trim_around_shared Zeng Heng 2022-08-30 15:12 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).