From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-131.freemail.mail.aliyun.com (out30-131.freemail.mail.aliyun.com [115.124.30.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9FE531F76D6 for ; Fri, 17 Jan 2025 09:28:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737106112; cv=none; b=n3PWHJ9HmHb88cReRy8yNq6dYCZn8QizTEaWmVHn5uCXCnFOvEj9l4844O7MAL+GwglOkID0+106mGhjh+/iVVmMqYSN4zRMY1jj7S9LvahtUyTNuwsEvlPKtxaBPC+ZaahET4F55/M7kezKzeyTh7f/gYs64/NxQ44ShLBLUBI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737106112; c=relaxed/simple; bh=etUZJ0g2Hb1kCS9hSrP9sJzyVCU8stjfuAsVY8q+Mao=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=nt+DTA7OOg08Pok+H5HnJmCRWjxPrijlY/s6wEG/CWDXm1/Qq5unOjGoBQcbGC/1u7QI2ERd86W7I1Y/Oyosk2tKKpjBFb9BVlkIjKkHCan/ZfOTPe48uB8ugHX5AAUDyu7LWbLXcu3lACcoXcWcr8HMsGUojce2XjY02hfC+RA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=NGALlIjJ; arc=none smtp.client-ip=115.124.30.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="NGALlIjJ" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1737106102; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=Iz21gsIr5e0wBKPF6pWFNo5BSeIFGVssbdWYguDsET8=; b=NGALlIjJQDbm6EHi21Q5nsp357sZUrbDpQAgMwug7yhafGIwqBjWb2NekJzwm/xIprMnNSmG/ctWlN71eEHCFG5Z00kl2pPVdbntw5nc9wguSL/2zmDZKrbFN3A4yK6AkwzPfhtBxNwLBXhz0mxGN7e1HGTSfGCWigJ+f9lHgLw= Received: from 30.41.10.74(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0WNoJmAa_1737106094 cluster:ay36) by smtp.aliyun-inc.com; Fri, 17 Jan 2025 17:28:21 +0800 Message-ID: <649afa9b-5724-4b52-8b9b-9a82a3c1468b@linux.alibaba.com> Date: Fri, 17 Jan 2025 17:28:13 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] erofs: add error log in erofs_fc_parse_param To: Chen Linxuan , Gao Xiang , Chao Yu , Yue Hu , Jeffle Xu , Sandeep Dhavale Cc: linux-erofs@lists.ozlabs.org, linux-kernel@vger.kernel.org References: From: Gao Xiang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Linxuan, On 2025/1/17 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. > > Entering `default` means `fs_parse` return something we unexpected. > I am not sure about it but I think we should return -EINVAL here, > just like `xfs_fs_parse_param`. > > Signed-off-by: Chen Linxuan I think the default branch is actually deadcode here, see erofs_fc_parse_param() -> fs_parse() -> fs_lookup_key() -> -ENOPARAM then vfs_parse_fs_param() will show "Unknown parameter". Maybe we could just kill `default:` branch... Thanks, Gao Xiang > --- > fs/erofs/super.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/erofs/super.c b/fs/erofs/super.c > index 1fc5623c3a4d..67fc4c1deb98 100644 > --- a/fs/erofs/super.c > +++ b/fs/erofs/super.c > @@ -509,7 +509,8 @@ static int erofs_fc_parse_param(struct fs_context *fc, > #endif > break; > default: > - return -ENOPARAM; > + errorfc(fc, "%s option not supported", param->key); > + return -EINVAL; > } > return 0; > }