| Message ID | 784ed247-0467-093b-1113-ff80a1289cbd@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs_db: more type_f cleanups | expand |
On Thu, Aug 20, 2020 at 07:05:37PM -0500, Eric Sandeen wrote: > There's no reason to go through the type change code if the > type has not been changed. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/db/type.c b/db/type.c > index 3cb1e868..572ac6d6 100644 > --- a/db/type.c > +++ b/db/type.c > @@ -216,6 +216,8 @@ type_f( > tt = findtyp(argv[1]); > if (tt == NULL) { > dbprintf(_("no such type %s\n"), argv[1]); > + } else if (iocur_top->typ == tt) { > + return 0; Doesn't this mean that verifier errors won't be printed if the user asks to set the type to the current type? e.g. xfs_db> agf 0 xfs_db> addr bnoroot xfs_db> fuzz -d level random Allowing fuzz of corrupted data with good CRC level = 59679 xfs_db> type bnobt Metadata corruption detected at 0x5586779a7b18, xfs_bnobt block 0x8/0x1000 xfs_db> type bnobt Metadata corruption detected at 0x5586779a7b18, xfs_bnobt block 0x8/0x1000 <shrug> OTOH, db doesn't consistently have that behavior either -- inodes only behave like that for crc errors, so maybe this is fine. Eh whatever, it's the debugging tool, you should be paying attention anyways. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > } else { > if (iocur_top->typ == NULL) > dbprintf(_("no current object\n")); >
On 8/21/20 9:46 AM, Darrick J. Wong wrote: > On Thu, Aug 20, 2020 at 07:05:37PM -0500, Eric Sandeen wrote: >> There's no reason to go through the type change code if the >> type has not been changed. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> diff --git a/db/type.c b/db/type.c >> index 3cb1e868..572ac6d6 100644 >> --- a/db/type.c >> +++ b/db/type.c >> @@ -216,6 +216,8 @@ type_f( >> tt = findtyp(argv[1]); >> if (tt == NULL) { >> dbprintf(_("no such type %s\n"), argv[1]); >> + } else if (iocur_top->typ == tt) { >> + return 0; > > Doesn't this mean that verifier errors won't be printed if the user asks > to set the type to the current type? e.g. > > xfs_db> agf 0 > xfs_db> addr bnoroot > xfs_db> fuzz -d level random > Allowing fuzz of corrupted data with good CRC > level = 59679 > xfs_db> type bnobt > Metadata corruption detected at 0x5586779a7b18, xfs_bnobt block 0x8/0x1000 > xfs_db> type bnobt > Metadata corruption detected at 0x5586779a7b18, xfs_bnobt block 0x8/0x1000 Oh, ok, I hadn't thought about how we could change the buffer after the fact, I was thinking "nothing should change" Honestly this isn't critical, it was a trivially backportable fix for the weird inode thing that Zorro fixed earlier, but with that in place it's not really necessary. I could go either way here too. If we want to rerun verifiers by resetting the type, that's fine. -Eric > <shrug> OTOH, db doesn't consistently have that behavior either -- > inodes only behave like that for crc errors, so maybe this is fine. > > Eh whatever, it's the debugging tool, you should be paying attention > anyways. > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > --D > >> } else { >> if (iocur_top->typ == NULL) >> dbprintf(_("no current object\n")); >> >
diff --git a/db/type.c b/db/type.c index 3cb1e868..572ac6d6 100644 --- a/db/type.c +++ b/db/type.c @@ -216,6 +216,8 @@ type_f( tt = findtyp(argv[1]); if (tt == NULL) { dbprintf(_("no such type %s\n"), argv[1]); + } else if (iocur_top->typ == tt) { + return 0; } else { if (iocur_top->typ == NULL) dbprintf(_("no current object\n"));
There's no reason to go through the type change code if the type has not been changed. Signed-off-by: Eric Sandeen <sandeen@redhat.com> ---