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

Christoph Hellwig
2012-07-02T06:26:28Z

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.