From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH 2/4] XFS: add op_flags field and helpers to xfs_da_args Date: Tue, 13 May 2008 04:34:58 -0400 Message-ID: <20080513083458.GB21919@infradead.org> References: <20080513075749.477238845@chook.melbourne.sgi.com> <20080513080152.598022276@chook.melbourne.sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: xfs@oss.sgi.com, linux-fsdevel@vger.kernel.org To: Barry Naujok Return-path: Received: from bombadil.infradead.org ([18.85.46.34]:42920 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757237AbYEMIe7 (ORCPT ); Tue, 13 May 2008 04:34:59 -0400 Content-Disposition: inline In-Reply-To: <20080513080152.598022276@chook.melbourne.sgi.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, May 13, 2008 at 05:57:51PM +1000, Barry Naujok wrote: > The end of the xfs_da_args structure has 4 unsigned char fields for > true/false information on directory and attr operations using the > xfs_da_args structure. > > The following converts these 4 into a op_flags field that uses the > first 4 bits for these fields and allows expansion for future > operation information (eg. case-insensitive lookup request). > > There is also a bit of EOL whitespace cleanup too. Looks generally good to me. A few stylistic comments: - I don't think the xfs_da_is*_op wrappers help readability, we'd be better off without those. - op_flags seems like a rather odd name to me, what about lookup_flags instead? And the hinks below are an awfull lot of random reformatting that don't belong into this patch. As they're sensible what about just commiting the beforehand? > dsize = dp->i_df.if_bytes; > - > + > switch (dp->i_d.di_format) { > case XFS_DINODE_FMT_EXTENTS: > - /* > - * If there is no attr fork and the data fork is extents, > - * determine if creating the default attr fork will result > - * in the extents form migrating to btree. If so, the > - * minimum offset only needs to be the space required for > + /* > + * If there is no attr fork and the data fork is extents, > + * determine if creating the default attr fork will result > + * in the extents form migrating to btree. If so, the > + * minimum offset only needs to be the space required for > * the btree root. > - */ > + */ > if (!dp->i_d.di_forkoff && dp->i_df.if_bytes > mp->m_attroffset) > dsize = XFS_BMDR_SPACE_CALC(MINDBTPTRS); > break; > - > + > case XFS_DINODE_FMT_BTREE: > /* > * If have data btree then keep forkoff if we have one, > - * otherwise we are adding a new attr, so then we set > - * minforkoff to where the btree root can finish so we have > + * otherwise we are adding a new attr, so then we set > + * minforkoff to where the btree root can finish so we have > * plenty of room for attrs > */ > if (dp->i_d.di_forkoff) { > - if (offset < dp->i_d.di_forkoff) > + if (offset < dp->i_d.di_forkoff) > return 0; > - else > + else > return dp->i_d.di_forkoff; > } else > dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot); > break; > } > - > - /* > - * A data fork btree root must have space for at least > + > + /* > + * A data fork btree root must have space for at least > * MINDBTPTRS key/ptr pairs if the data fork is small or empty. > */ > minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS));