On Thu, Oct 12, 2023 at 07:39:54AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 11, 2023 at 11:07:01AM -0700, Darrick J. Wong wrote:
> > +/* Return a pointer to a bitmap word within a rt bitmap block buffer. */
> > +static inline xfs_rtword_t *
> > +xfs_rbmbuf_wordptr(
> > + void *buf,
> > + unsigned int rbmword)
> > +{
> > + xfs_rtword_t *wordp = buf;
> > +
> > + return &wordp[rbmword];
>
> Superficial nitpick, I find the array dereference syntax here highly
> confusing as the passed in pointer is not an array at all.
>
> In fact I wonder what the xfs_rbmbuf_wordptr helper is for? Even
> looking at your full patch stack xfs_rbmblock_wordptr seems like
> the only user.
IIRC an earlier version of this patchset actually tried to use
xfs_rbmbuf_wordptr in the online repair code when it was building the
new rtbitmap file contents. At some point I must've decided that it was
easier to access the xfile directly.
So, yes, this helper can be deleted.
> > +/* Return a pointer to a bitmap word within a rt bitmap block. */
> > +static inline xfs_rtword_t *
> > +xfs_rbmblock_wordptr(
> > + struct xfs_buf *bp,
> > + unsigned int rbmword)
> > +{
> > + return xfs_rbmbuf_wordptr(bp->b_addr, rbmword);
>
> So I'd much rather just open code this as:
>
> xfs_rtword_t *words = bp->b_addr;
>
> return words + rbmword;
>
> and if I want to get really fancy I'd maybe rename rbmword to
> something like index which feels more readable than rbmword.
Done.
> That beeing said the xfs_rbmblock_wordptr abstraction is very welcome.
Thanks!
--D
