public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <aelder@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 14/27] xfs: kill struct xfs_dir2_sf
Date: Wed, 6 Jul 2011 10:05:34 -0500	[thread overview]
Message-ID: <1309964734.1931.42.camel@doink> (raw)
In-Reply-To: <20110706083345.GB19861@infradead.org>

On Wed, 2011-07-06 at 04:33 -0400, Christoph Hellwig wrote:
> 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@lst.de>
> > 
> > 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.

Understood.

> > > +	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.

It was simply because you already assigned oldsfp
the value you were asserting was null.  Your way
states something about the source value though,
so I guess it more directly states the condition
you're assuming here.

> > >  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.

No pressing need.  If you're inspired to do it, fine, but
it's readable despite the inconsistency.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2011-07-06 15:05 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-01  9:43 [PATCH 00/27] patch queue for Linux 3.1, V2 Christoph Hellwig
2011-07-01  9:43 ` [PATCH 01/27] xfs: PF_FSTRANS should never be set in ->writepage Christoph Hellwig
2011-07-01  9:43 ` [PATCH 02/27] xfs: re-enable non-blocking behaviour in xfs_map_blocks Christoph Hellwig
2011-07-05 22:35   ` Alex Elder
2011-07-06  6:37     ` Christoph Hellwig
2011-07-06 13:36       ` Alex Elder
2011-07-01  9:43 ` [PATCH 03/27] xfs: work around bogus gcc warning in xfs_allocbt_init_cursor Christoph Hellwig
2011-07-01  9:43 ` [PATCH 04/27] xfs: split xfs_setattr Christoph Hellwig
2011-07-01  9:43 ` [PATCH 06/27] xfs: kill xfs_itruncate_start Christoph Hellwig
2011-07-01  9:43 ` [PATCH 07/27] xfs: split xfs_itruncate_finish Christoph Hellwig
2011-07-06  4:35   ` Alex Elder
2011-07-06  8:11     ` Christoph Hellwig
2011-07-06 14:05       ` Alex Elder
2011-07-01  9:43 ` [PATCH 08/27] xfs: improve sync behaviour in the fact of aggressive dirtying Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-06  8:15     ` Christoph Hellwig
2011-07-06 14:59       ` Alex Elder
2011-07-01  9:43 ` [PATCH 09/27] xfs: fix filesystsem freeze race in xfs_trans_alloc Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 10/27] xfs: remove i_transp Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 11/27] xfs: kill the unused struct xfs_sync_work Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 12/27] xfs: factor out xfs_dir2_leaf_find_entry Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 13/27] xfs: cleanup shortform directory inode number handling Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 14/27] xfs: kill struct xfs_dir2_sf Christoph Hellwig
2011-07-06  1:57   ` Dave Chinner
2011-07-06  8:28     ` Christoph Hellwig
2011-07-06  3:24   ` Alex Elder
2011-07-06  8:33     ` Christoph Hellwig
2011-07-06 15:05       ` Alex Elder [this message]
2011-07-01  9:43 ` [PATCH 15/27] xfs: cleanup the defintion of struct xfs_dir2_sf_entry Christoph Hellwig
2011-07-06  2:00   ` Dave Chinner
2011-07-06  3:33   ` Alex Elder
2011-07-06  8:34     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 16/27] xfs: avoid usage of struct xfs_dir2_block Christoph Hellwig
2011-07-06  2:19   ` Dave Chinner
2011-07-06  8:35     ` Christoph Hellwig
2011-07-06  3:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 17/27] xfs: kill " Christoph Hellwig
2011-07-06  2:31   ` Dave Chinner
2011-07-06  8:37     ` Christoph Hellwig
2011-07-06 15:11       ` Alex Elder
2011-07-06  3:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data Christoph Hellwig
2011-07-06  3:02   ` Dave Chinner
2011-07-06  8:43     ` Christoph Hellwig
2011-07-06  3:38   ` Alex Elder
2011-07-06  8:45     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 19/27] xfs: kill " Christoph Hellwig
2011-07-06  3:05   ` Dave Chinner
2011-07-06  3:38   ` Alex Elder
2011-07-01  9:43 ` [PATCH 20/27] xfs: cleanup the defintion of struct xfs_dir2_data_entry Christoph Hellwig
2011-07-06  3:06   ` Dave Chinner
2011-07-06  3:44   ` Alex Elder
2011-07-06  8:48     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 21/27] xfs: cleanup struct xfs_dir2_leaf Christoph Hellwig
2011-07-06  3:13   ` Dave Chinner
2011-07-06  3:44   ` Alex Elder
2011-07-01  9:43 ` [PATCH 22/27] xfs: use generic get_unaligned_beXX helpers Christoph Hellwig
2011-07-06  3:44   ` Dave Chinner
2011-07-06  9:07     ` Christoph Hellwig
2011-07-07  8:00       ` Christoph Hellwig
2011-07-06  3:47   ` Alex Elder
2011-07-01  9:43 ` [PATCH 23/27] xfs: remove the unused xfs_bufhash structure Christoph Hellwig
2011-07-06  3:44   ` Dave Chinner
2011-07-06  3:49   ` Alex Elder
2011-07-01  9:43 ` [PATCH 24/27] xfs: clean up buffer locking helpers Christoph Hellwig
2011-07-06  3:47   ` Dave Chinner
2011-07-06  3:55   ` Alex Elder
2011-07-01  9:43 ` [PATCH 25/27] xfs: return the buffer locked from xfs_buf_get_uncached Christoph Hellwig
2011-07-06  3:48   ` Dave Chinner
2011-07-06  3:57   ` Alex Elder
2011-07-01  9:43 ` [PATCH 26/27] xfs: cleanup I/O-related buffer flags Christoph Hellwig
2011-07-06  3:54   ` Dave Chinner
2011-07-06  9:11     ` Christoph Hellwig
2011-07-06  4:09   ` Alex Elder
2011-07-06  9:11     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 27/27] xfs: avoid a few disk cache flushes Christoph Hellwig
2011-07-06  3:55   ` Dave Chinner
2011-07-06  4:11   ` Alex Elder
2011-07-06  4:40 ` [PATCH 00/27] patch queue for Linux 3.1, V2 Alex Elder
2011-07-06  6:42   ` Christoph Hellwig
2011-07-06 13:32     ` Alex Elder
2011-07-06 13:43       ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1309964734.1931.42.camel@doink \
    --to=aelder@sgi.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox