On 8/21/20 9:52 AM, Darrick J. Wong wrote: > On Thu, Aug 20, 2020 at 07:09:45PM -0500, Eric Sandeen wrote: >> Right now there are 3 cases to type_f: inode type, type with fields, >> and a default. The first two were added to address issues with handling >> V5 metadata. >> >> The first two already use some version of set_cur, which handles all >> of the validation etc. There's no reason to leave the open-coded bits >> at the end, just send every non-inode type through set_cur and be done >> with it. >> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >> --- >> >> io.c | 28 +++++----------------------- >> 1 file changed, 5 insertions(+), 23 deletions(-) >> >> diff --git a/db/io.c b/db/io.c >> index 884da599..235191f5 100644 >> --- a/db/io.c >> +++ b/db/io.c >> @@ -603,33 +603,15 @@ set_iocur_type( >> iocur_top->boff / mp->m_sb.sb_inodesize); >> ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b), agino); >> set_cur_inode(ino); >> - return; >> - } >> - >> - /* adjust buffer size for types with fields & hence fsize() */ >> - if (type->fields) { >> - int bb_count; /* type's size in basic blocks */ >> + } else { > > Two spaces ^^ between the else and the paren. > > I also wonder, why not leave the "return;" at the end of the > "if (type->typnm == TYP_INODE)" part and then you can reduce the > indenting level of the rest of the function? Oh, that might be nice. I'll see how that looks. > <shrug> maintainer's prerogative on that one though. > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > /me also wonders why dquots don't get the same "save the buffer offset" > treatment as inodes, but that's a separate question. :) hmmmm good question tho. Thanks for the reviews, -Eric > --D > >> + int bb_count = 1; /* type's size in basic blocks */ >> >> - bb_count = BTOBB(byteize(fsize(type->fields, >> + /* adjust buffer size for types with fields & hence fsize() */ >> + if (type->fields) >> + bb_count = BTOBB(byteize(fsize(type->fields, >> iocur_top->data, 0, 0))); >> set_cur(type, iocur_top->bb, bb_count, DB_RING_IGN, NULL); >> } >> - iocur_top->typ = type; >> - >> - /* verify the buffer if the type has one. */ >> - if (!bp) >> - return; >> - if (!type->bops) { >> - bp->b_ops = NULL; >> - bp->b_flags |= LIBXFS_B_UNCHECKED; >> - return; >> - } >> - if (!(bp->b_flags & LIBXFS_B_UPTODATE)) >> - return; >> - bp->b_error = 0; >> - bp->b_ops = type->bops; >> - bp->b_ops->verify_read(bp); >> - bp->b_flags &= ~LIBXFS_B_UNCHECKED; >> } >> >> static void >> >