On Thu, Mar 21, 2019 at 4:48 AM David Howells <dhowells@xxxxxxxxxx> wrote: > > Signed-off-by: David Howells <dhowells@xxxxxxxxxx> > cc: Kees Cook <keescook@xxxxxxxxxxxx> > cc: Anton Vorontsov <anton@xxxxxxxxxx> > cc: Colin Cross <ccross@xxxxxxxxxxx> > cc: Tony Luck <tony.luck@xxxxxxxxx> Thanks for doing this conversion! Acked-by: Kees Cook <keescook@xxxxxxxxxxxx> Some questions/nits below: > +static int pstore_parse_param(struct fs_context *fc, struct fs_parameter *param) > +{ > + struct pstore_fs_context *ctx = fc->fs_private; > + struct fs_parse_result result; > + int opt; > + > + opt = fs_parse(fc, &pstore_fs_parameters, param, &result); > + if (opt < 0) > + return opt; > + > + switch (opt) { > + case Opt_kmsg_bytes: > + ctx->kmsg_bytes = result.uint_32; > + break; > + } > > - while ((p = strsep(&options, ",")) != NULL) { > - int token; > + return 0; > +} > > - if (!*p) > - continue; > +static void pstore_apply_param(struct fs_context *fc) > +{ > + struct pstore_fs_context *ctx = fc->fs_private; > > - token = match_token(p, tokens, args); > - switch (token) { > - case Opt_kmsg_bytes: > - if (!match_int(&args[0], &option)) > - pstore_set_kmsg_bytes(option); > - break; > - } > - } > + pstore_set_kmsg_bytes(ctx->kmsg_bytes); > } Why the separation between parse and apply now? Is this due to the reconfigure calls? (i.e. why not call pstore_set_kmsg_bytes() in pstore_parse_param()? > @@ -416,14 +423,38 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent) > return -ENOMEM; > > pstore_get_records(0); > - > return 0; > } I prefer to keep a blank before "return", but no need to drop this unless it gets a respin. Thanks again! -- Kees Cook