xfs
[Top] [All Lists]

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

To: Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx>
Subject: Re: [PATCH 1/2 V2] xfs: cleanup the mount options
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 2 Jul 2012 02:26:25 -0400
Cc: XFS <xfs@xxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Ben Myers <bpm@xxxxxxx>, Zach Brown <zab@xxxxxxxxx>
In-reply-to: <1340993202-5560-1-git-send-email-gaowanlong@xxxxxxxxxxxxxx>
References: <1340816243-6177-1-git-send-email-gaowanlong@xxxxxxxxxxxxxx> <1340993202-5560-1-git-send-email-gaowanlong@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxx>
> CC: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> CC: Dave Chinner <david@xxxxxxxxxxxxx>
> CC: Zach Brown <zab@xxxxxxxxx>
> Signed-off-by: Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx>
> ---
>  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.

<Prev in Thread] Current Thread [Next in Thread>