On Tue, Nov 05, 2019 at 02:44:03AM +0100, Christoph Hellwig wrote:
> On Mon, Nov 04, 2019 at 12:21:45PM -0800, Darrick J. Wong wrote:
> > > @@ -233,6 +233,7 @@ xfs_dir2_free_hdr_from_disk(
> > > to->firstdb = be32_to_cpu(from3->hdr.firstdb);
> > > to->nvalid = be32_to_cpu(from3->hdr.nvalid);
> > > to->nused = be32_to_cpu(from3->hdr.nused);
> > > + to->bests = (void *)from3 + sizeof(struct xfs_dir3_free_hdr);
> >
> > Urgh, isn't void pointer arithmetic technically illegal according to C?
>
> It is not specified in ISO C, but clearly specified in the GNU C
> extensions and used all over the kernel.
Just out of curiosity, do you know if clang supports that extension?
Once in a while we get patches from them to fix various clang warnings,
and at least as of a few years ago clang got grouchy about void pointer
arithmetic.
> > In any case, shouldn't this cast through struct xfs_dir3_free instead of
> > open-coding details of the disk format that we've already captured? The
> > same question also applies to the other patches that add pointers to
> > ondisk leaf and intnode pointers into the incore header struct.
>
> I don't really understand that sentence. What would do you instead?
if (xfs_sb_version_hascrc(&mp->m_sb)) {
struct xfs_dir3_free *from3 = (struct xfs_dir3_free *)from;
...
to->nused = be32_to_cpu(from3->hdr.nused);
to->bests = &from3->bests[0];
}
Since we're already passing around pointers to the xfs_dir[23]_free
structure, we might as well use it instead of open-coding the arithmetic.
Sorry that wasn't clear. :/
--D
