Re: [PATCH] erofs: add error log in erofs_fc_parse_param

From: Gao Xiang
Date: Sat Jan 18 2025 - 03:45:52 EST




On 2025/1/18 09:25, Ian Kent wrote:
On 18/1/25 08:45, Ian Kent wrote:
On 17/1/25 16:52, Chen Linxuan wrote:
While reading erofs code, I notice that `erofs_fc_parse_param` will
return -ENOPARAM, which means that erofs do not support this option,
without report anything when `fs_parse` return an unknown `opt`.

But if an option is unknown to erofs, I mean that option not in
`erofs_fs_parameters` at all, `fs_parse` will return -ENOPARAM,
which means that `erofs_fs_parameters` should has returned earlier.

I'm pretty sure than the vfs deals with reporting unknown options

and returns -EINVAL already.


I think the caller oferofs_fc_parse_param() is vfs_parse_fs_param()

and for an -ENOPARAM return will ultimately do this:

return invalf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);

which does this.

The thing about this is the mount API macro deals with (or it should,

although I'm not sure that's completely sorted out yet) logging the

message to the console log as well as possibly making it available to

mount api system calls. I'm pretty sure this change will prevent the

error message being available for mount api system calls to retrieve.

Actually no, removing the default switch branch does look like the right thing

to do, my mistake. The call to fs_parse() will return -NOPARAM so the

switch doesn't need to handle that case.


Oops!

Yeah, actually dead code..

Thanks,
Gao Xiang



Ian