From: Al Viro <viro@zeniv.linux.org.uk>
To: Zhen Ni <zhen.ni@easystack.cn>
Cc: rmk+kernel@armlinux.org.uk, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs/adfs: bigdir: Restore EIO errno return when checkbyte mismatch
Date: Fri, 1 Aug 2025 20:22:00 +0100 [thread overview]
Message-ID: <20250801192200.GR222315@ZenIV> (raw)
In-Reply-To: <20250801093806.1223287-1-zhen.ni@easystack.cn>
On Fri, Aug 01, 2025 at 05:38:06PM +0800, Zhen Ni wrote:
> Fix smatch warn:
> fs/adfs/dir_fplus.c:146 adfs_fplus_read() warn: missing error code 'ret'
Gyah... You don't fix $TOOL warnings, you fix underlying bugs - or, in case
of $TOOL generating a false positive, making $TOOL to STFU. At the very
least you need to tell which one it is.
In this case it was not a false positive, so you need an explanation of
what's wrong with the code in question ("it makes smatch to complain"
is *NOT* it).
What happens here is a failure exit where we do complain into dmesg,
but actually report success to the caller, since the eventual return
value (in 'ret') is 0 left behind by successful adfs_fplus_validate_tail()
in the previous test.
_IF_ that behaviour had been an intentional mitigation strategy,
it needs to be explicitly described as such; however, looking at the
history of that function it seems more likely that this is an analogue
of the bug in previous failure exit fixed in 587065dcac64 ("fs/adfs:
bigdir: Fix an error code in adfs_fplus_read()"), so it's probably
better to return an error, same as we do on the other failure exits.
As it is, commit message fails to explain anything other than "smatch
has been involved in catching this one". If you want to mention that,
sure, but it's on the same level as "so-and-so told me this place
looks fishy" - by all means, credit the contribution. Reported-by/
caught-by/actually quoting smatch warning somewhere in the message -
up to you, but that shouldn't be the entirety of commit message ;-)
next prev parent reply other threads:[~2025-08-01 19:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-01 9:38 [PATCH] fs/adfs: bigdir: Restore EIO errno return when checkbyte mismatch Zhen Ni
2025-08-01 13:25 ` Markus Elfring
2025-08-01 19:22 ` Al Viro [this message]
2025-08-01 21:20 ` Russell King (Oracle)
2025-08-05 7:30 ` [PATCH v2] " Zhen Ni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250801192200.GR222315@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=rmk+kernel@armlinux.org.uk \
--cc=zhen.ni@easystack.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.