Re: [PATCH 1/5] xfs: make superblock version checks reflect reality

[Christoph Hellwig <hch@infradead.org>]

> +/*
> + * The first XFS version we support is a v4 superblock with V2 directories.
> + */
> +static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
>  {
> +	if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
> +		return false;
>  
> +	/* check for unknown features in the fs */
> +	if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) ||

Given that sb_versionnum is a __uint16_t and XFS_SB_VERSION_OKBITS is
0xffff this will never evaluate to false and a sane compiler should warn
about it.  How about remove this check and XFS_SB_VERSION_OKBITS?

The various has_ macros are a bit confusing to me, as some explicitly
check for 5 superblocks, and some assume the caller handles them in
some way, but I think this is something we can leave for later cleanups.

>   * For example, for a bit defined as XFS_SB_VERSION2_FUNBIT, has a macro:
>   *
> - * SB_VERSION_HASFUNBIT(xfs_sb_t *sbp)
> + * SB_VERSION_HASFUNBIT(struct xfs_sb *sbp)

>   *	((xfs_sb_version_hasmorebits(sbp) &&
>   *	 ((sbp)->sb_features2 & XFS_SB_VERSION2_FUNBIT)
>   */

This should be updated to the lowe case convention inlines we've used
for a long time.  Or just removed as new features should go into v5
superblocks..

Modulo these minor bits:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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