Re: [PATCH 5/5] xfs: reinitialize rm_flags when unpacking an offset into an rmap irec

From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 5/5] xfs: reinitialize rm_flags when unpacking an offset into an rmap irec
Date: Fri, 30 Aug 2019 08:31:31 -0700	[thread overview]
Message-ID: <20190830153131.GB13924@infradead.org> (raw)
In-Reply-To: <20190829160118.GG5354@magnolia>

On Thu, Aug 29, 2019 at 09:01:18AM -0700, Darrick J. Wong wrote:
> > >  	irec->rm_owner = be64_to_cpu(rec->rmap.rm_owner);
> > > diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h
> > > index 0c2c3cb73429..abe633403fd1 100644
> > > --- a/fs/xfs/libxfs/xfs_rmap.h
> > > +++ b/fs/xfs/libxfs/xfs_rmap.h
> > > @@ -68,6 +68,7 @@ xfs_rmap_irec_offset_unpack(
> > >  	if (offset & ~(XFS_RMAP_OFF_MASK | XFS_RMAP_OFF_FLAGS))
> > >  		return -EFSCORRUPTED;
> > >  	irec->rm_offset = XFS_RMAP_OFF(offset);
> > > +	irec->rm_flags = 0;
> > 
> > The change looks sensible-ish.  But why do we even have a separate
> > xfs_rmap_irec_offset_unpack with a single caller nd out of the
> > way in a header?  Wouldn't it make sense to just merge the two
> > functions?
> 
> xfs_repair uses libxfs_rmap_irec_offset_unpack, which is why it's a
> separate function.

True.  Athough repair would also benefit a lot from a version of
xfs_rmap_btrec_to_irec that just takes a bmbt_key instead, but that is
for another time.