xfs
[Top] [All Lists]

Re: [PATCH 14/27] xfs: kill struct xfs_dir2_sf

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 14/27] xfs: kill struct xfs_dir2_sf
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 6 Jul 2011 04:33:45 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <1309922658.3381.22.camel@doink>
References: <20110701094321.936534538@xxxxxxxxxxxxxxxxxxxxxx> <20110701094605.197942925@xxxxxxxxxxxxxxxxxxxxxx> <1309922658.3381.22.camel@doink>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jul 05, 2011 at 10:24:18PM -0500, Alex Elder wrote:
> On Fri, 2011-07-01 at 05:43 -0400, Christoph Hellwig wrote:
> > The list field of it is never cactually used, so all uses can simply be
> > replaced with the xfs_dir2_sf_hdr_t type that it has as first member.
> > 
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> Looks like a lot of places could be converted to use
> "struct xfs_dir2_sf_hdr" rather than the typedef, but
> it's not worth re-posting for that.  (Plus I suspect
> such changes may be in forthcoming patches...)

In general they should, but I try to avoid that where it means
massive formatting changes, as that just clutters up the patch.

> > +   oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> > +
> >     ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
> >     ASSERT(dp->i_df.if_u1.if_data != NULL);
> 
>       ASSERT(oldsfp != NULL);

What for?  We'll just dereference it later anyway.

> >  static xfs_ino_t
> >  xfs_dir2_sf_get_ino(
> > -   struct xfs_dir2_sf      *sfp,
> > +   struct xfs_dir2_sf_hdr  *hdr,
> 
> I think I like the name "hdr" better than "sfp";
> was it just too widespread a change to do a
> similar rename elsewhere?  (xfs_dir2_block_to_sf()
> uses "sfhp" already, though I like just "hdr".)

Yeah, I tried to keep the change small in general.  If people like it
I can do a big sweep to convert stuff to struct types and common names
as a follow-on.

<Prev in Thread] Current Thread [Next in Thread>