From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-124.freemail.mail.aliyun.com (out30-124.freemail.mail.aliyun.com [115.124.30.124]) (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 45A071FC7ED for ; Fri, 17 Jan 2025 10:08:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737108524; cv=none; b=N4xCj+Y8tY5kaTs/x/qsJSTOnk4e309Hdbv8iqYpdNZuhOefUUX+qqJHGqfefpwdFndppjTuQGCX6jValEKTqd5vG6asy3CTvckLrxKQoOOxMXwYG3u2nSbkLN9XUI7VO0xMVZUV8zDBEc0NVRDOYbguDq5TvNaNJZSUVon1Wi4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737108524; c=relaxed/simple; bh=NKpcRhyv2TQOTo18dPeCVSIRk7UKFH0JEdP+zmKgyxw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Q5SuTYWYK1gs82DxqLbWZZgZn4aOaVZsQaXZLLsTw/dhHaLJ3YatPirY6ZFg5/THLzxo/4Zvfno4i5+HxE0uUdIO7gey+o/KhRTaUtOzcAI/1AYQaMjD8kHs3c9XqAXN6O9yRV+B9DmENtOtPSiyRDGRw7Di9pOSxYaTtuNI7G4= 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=NSJZTXgZ; arc=none smtp.client-ip=115.124.30.124 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="NSJZTXgZ" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1737108513; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=e3EHTj+CPpNUPdTU5v9K5KYvPWG8yi1EwhqmuizUdC4=; b=NSJZTXgZrXfeNgozyTJOSeSIQEGsMXTw7/r+9mb/I6MsL/aZJegG9a5FXav9Mt1gc8++C9E3HpdsNELLOxotTfxJk7+ohfNf/dMcKYo3eVIdnq5lGSZCaAyoTo+7vzc7wiyjI0Q5DA8x6B92ZU4x2eAx8Gx3f9XOV1UogUI3ViI= Received: from 30.41.10.74(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0WNoTYjm_1737108505 cluster:ay36) by smtp.aliyun-inc.com; Fri, 17 Jan 2025 18:08:32 +0800 Message-ID: <2ac05d5d-1e9d-448e-99e8-64ebefdc8c55@linux.alibaba.com> Date: Fri, 17 Jan 2025 18:08:23 +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: <649afa9b-5724-4b52-8b9b-9a82a3c1468b@linux.alibaba.com> <640C401CAB291F86+ffb78b4f37e75faf2b4730e625b8d72d15be782a.camel@uniontech.com> <58cadb57-22ce-4818-af2b-9ae452c38f27@linux.alibaba.com> From: Gao Xiang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2025/1/17 18:00, Chen Linxuan wrote: > On Fri, 2025-01-17 at 17:54 +0800, Gao Xiang wrote: >> >> On 2025/1/17 17:50, Chen Linxuan wrote: >>> On Fri, 2025-01-17 at 17:28 +0800, Gao Xiang wrote: >>>> 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... >>> >>> ext4 do not have a `default:` branch, but xfs return -EINVAL. >>> >>> I think `default:` branch can report error when `fs_parse` or >>> `erofs_fs_parameters` goes wrong. >> >> How can it go wrong? > > What if we forget to update the switch branch for a new option? Then it's clearly a bug (we don't even handle the new option), I think we shouldn't consider it as a normal case. Thanks, Gao Xiang > >> >> Thanks, >> Gao Xiang >> >>