Re: [PATCH] btrfs: rename the option to nospace_cache

[Roman Mamedov <rm@romanrm.ru>]

[-- Attachment #1: Type: text/plain, Size: 2107 bytes --]

On Fri, 11 Nov 2011 16:41:55 +0100
David Sterba <dsterba@suse.cz> wrote:

> Rename no_space_cache option to nospace_cache to be more consistent with
> the rest, where the simple prefix 'no' is used to negate an option.
> 
> The option has been introduced during the -rc1 cycle and there are has not been
> widely used, so it's safe.

In my opinion as soon as you start using underscores to split words in an identifier, you have to use them to separate all the words, not just some of them. So it should be either no_space_cache, or nospacecache; the version proposed here looks puzzling and wrong ("does this enable a 'cache for nospace'?").

Also, space_cache=off would be vastly better IMO. Consistent with compress=blah/compress-force=blah, too. And in the same manner theoretically there could be several algorithms for space caching in future, switchable by this same option ("space_cache=whatever").

> 
> Signed-off-by: David Sterba <dsterba@suse.cz>
> ---
> 
>  fs/btrfs/super.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 629281c..8bd9d6d 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -197,7 +197,7 @@ static match_table_t tokens = {
>  	{Opt_subvolrootid, "subvolrootid=%d"},
>  	{Opt_defrag, "autodefrag"},
>  	{Opt_inode_cache, "inode_cache"},
> -	{Opt_no_space_cache, "no_space_cache"},
> +	{Opt_no_space_cache, "nospace_cache"},
>  	{Opt_recovery, "recovery"},
>  	{Opt_err, NULL},
>  };
> @@ -711,7 +711,7 @@ static int btrfs_show_options(struct seq_file *seq, struct vfsmount *vfs)
>  	if (btrfs_test_opt(root, SPACE_CACHE))
>  		seq_puts(seq, ",space_cache");
>  	else
> -		seq_puts(seq, ",no_space_cache");
> +		seq_puts(seq, ",nospace_cache");
>  	if (btrfs_test_opt(root, CLEAR_CACHE))
>  		seq_puts(seq, ",clear_cache");
>  	if (btrfs_test_opt(root, USER_SUBVOL_RM_ALLOWED))


-- 
With respect,
Roman

~~~~~~~~~~~~~~~~~~~~~~~~~~~
"Stallman had a printer,
with code he could not see.
So he began to tinker,
and set the software free."

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]