On Tue, Apr 19, 2022 at 06:56:00PM +0200, Andrey Albershteyn wrote:
> On Tue, Apr 19, 2022 at 08:47:50AM -0700, Darrick J. Wong wrote:
> > On Tue, Apr 19, 2022 at 02:19:51PM +0200, Andrey Albershteyn wrote:
> > > Changing the interpretation type of data under the cursor moves the
> > > cursor to the beginning of BB cluster. When cursor is set to an
> > > inode the cursor is offset in BB buffer. However, this offset is not
> > > considered when type of the data is changed - the cursor points to
> > > the beginning of BB buffer. For example:
> > >
> > > $ xfs_db -c "inode 131" -c "daddr" -c "type text" \
> > > -c "daddr" /dev/sdb1
> > > current daddr is 131
> > > current daddr is 128
> > >
> > > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> > > ---
> > > db/io.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/db/io.c b/db/io.c
> > > index df97ed91..107f2e11 100644
> > > --- a/db/io.c
> > > +++ b/db/io.c
> > > @@ -589,6 +589,7 @@ set_iocur_type(
> > > const typ_t *type)
> > > {
> > > int bb_count = 1; /* type's size in basic blocks */
> > > + int boff = iocur_top->boff;
> >
> > Nit: Please line up the variable names.
>
> sure ;)
>
> >
> > >
> > > /*
> > > * Inodes are special; verifier checks all inodes in the chunk, the
> > > @@ -613,6 +614,9 @@ set_iocur_type(
> > > 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->boff = boff;
> > > + iocur_top->off = ((xfs_off_t)iocur_top->bb << BBSHIFT) + boff;
> > > + iocur_top->data = (void *)((char *)iocur_top->buf + boff);
> >
> > It seems reasonable to me to preserve the io cursor's boff when we're
> > setting /only/ the buffer type, but this function and off_cur() could
> > share these three lines of code that (re)set boff/off/data.
> >
> > Alternately, I guess you could just call off_cur(boff, BBTOB(bb_count))
> > here.
>
> This won't pass the second condition in off_cur(). I suppose the
> purpose of off_cur() was to shift io cursor in BB buffer. But
> changing the type changes the blen which could be smaller (e.g.
> inode blen == 32 -> text blen == 1). Anyway, will try to come up
> with a meaningful name for this 3 lines function :)
Ooh, a bikeshed! How about:
static inline void set_cur_boff(int boff)
{
iocur_top->boff = boff;
iocur_top->off = ((xfs_off_t)iocur_top->bb << BBSHIFT) + boff;
iocur_top->data = (void *)((char *)iocur_top->buf + boff);
}
> I think the other solution could be to set boff in set_cur(), but
> this will require more refactoring and I suppose this is the only
> place where newly added argument would be used.
>
> >
> > --D
> >
> > > }
> > >
> > > static void
> > > --
> > > 2.27.0
> > >
> >
>
> --
> - Andrey
>
