Re: [PATCH 1/2 V2] xfs: cleanup the mount options

[Christoph Hellwig <hch@infradead.org>]

On Sat, Jun 30, 2012 at 02:06:41AM +0800, Wanlong Gao wrote:
> remove the mount options macro, use tokens instead.
> 
> CC: Ben Myers <bpm@sgi.com>
> CC: Christoph Hellwig <hch@infradead.org>
> CC: Dave Chinner <david@fromorbit.com>
> CC: Zach Brown <zab@zabbo.net>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
>  fs/xfs/xfs_super.c |  523 ++++++++++++++++++++++++++++------------------------
>  1 file changed, 287 insertions(+), 236 deletions(-)

This looks pretty good, some minor comments below:

>  /*
>   * Table driven mount option parser.
>   */

This comment can go now that there is only a single mount option parser.

> +free_string:
> +	kfree(string);
> +	string = NULL;
> +free_orig:
> +	kfree(orig);
> +	return ret;
> +}

no need to set string to NULL just before returning.

> -	static struct proc_xfs_info xfs_info_set[] = {
> -		/* the few simple ones we can get from the mount struct */
> -		{ XFS_MOUNT_IKEEP,		"," MNTOPT_IKEEP },
> -		{ XFS_MOUNT_WSYNC,		"," MNTOPT_WSYNC },
> -		{ XFS_MOUNT_NOALIGN,		"," MNTOPT_NOALIGN },
> -		{ XFS_MOUNT_SWALLOC,		"," MNTOPT_SWALLOC },
> -		{ XFS_MOUNT_NOUUID,		"," MNTOPT_NOUUID },
> -		{ XFS_MOUNT_NORECOVERY,		"," MNTOPT_NORECOVERY },
> -		{ XFS_MOUNT_ATTR2,		"," MNTOPT_ATTR2 },
> -		{ XFS_MOUNT_FILESTREAMS,	"," MNTOPT_FILESTREAM },
> -		{ XFS_MOUNT_GRPID,		"," MNTOPT_GRPID },
> -		{ XFS_MOUNT_DISCARD,		"," MNTOPT_DISCARD },
> -		{ 0, NULL }
> -	};
> -	static struct proc_xfs_info xfs_info_unset[] = {
> -		/* the few simple ones we can get from the mount struct */
> -		{ XFS_MOUNT_COMPAT_IOSIZE,	"," MNTOPT_LARGEIO },
> -		{ XFS_MOUNT_BARRIER,		"," MNTOPT_NOBARRIER },
> -		{ XFS_MOUNT_SMALL_INUMS,	"," MNTOPT_64BITINODE },
> -		{ 0, NULL }
> -	};

I can't find any good reason to remove these tables, just replace the
MNTOPT_ constants with the plain strings.

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