Re: [RFC PATCH 4/4] 9p: convert to the new mount API

[David Howells <dhowells@redhat.com>]

Eric Sandeen <sandeen@redhat.com> wrote:

> +	case Opt_uname:
> +		kfree(v9ses_opts->uname);
> +		v9ses_opts->uname = param->string;
> +		param->string = NULL;

I wonder if it's worth using swap() for that:

		v9ses_opts->uname = swap(param->string, NULL);

> +		if (strcmp(s, "user") == 0)
> +			v9ses_opts->flags |= V9FS_ACCESS_USER;
> +		else if (strcmp(s, "any") == 0)
> +			v9ses_opts->flags |= V9FS_ACCESS_ANY;
> +		else if (strcmp(s, "client") == 0) {
> +			v9ses_opts->flags |= V9FS_ACCESS_CLIENT;
> +		} else {

Maybe put braces on all the clauses in that?

> +static void v9fs_free_fc(struct fs_context *fc)
> +{
> +	struct v9fs_context *ctx = fc->fs_private;

I would check that ctx isn't NULL.

> +static int v9fs_init_fs_context(struct fs_context *fc)
> +{
> +	struct v9fs_context	*ctx;
> +
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	/* initialize core options */
> +	ctx->v9ses.afid = ~0;
> +	ctx->v9ses.debug = 0;
> +	ctx->v9ses.cache = CACHE_NONE;
> +#ifdef CONFIG_9P_FSCACHE
> +	ctx->v9ses.cachetag = NULL;
> +#endif

You used kzalloc... ;-).

> +err_names:
> +	kfree(ctx->v9ses.uname);
> +	kfree(ctx->v9ses.aname);
> +	kfree(ctx);
> +	return -ENOMEM;
> +}

Instead of doing the freeing, you could just set ->need_free and if
->init_fs_context() returns an error, the context will be put and that will
invoke ->free().

David