diff mbox series

badblocks: Fix a nonsense WARN_ON() which checks whether a u64 variable < 0

Message ID 20250309160556.42854-1-colyli@kernel.org (mailing list archive)
State New
Headers show
Series badblocks: Fix a nonsense WARN_ON() which checks whether a u64 variable < 0 | expand

Commit Message

Coly Li March 9, 2025, 4:05 p.m. UTC 
From: Coly Li <colyli@kernel.org>

In _badblocks_check(), there are lines of code like this,
1246         sectors -= len;
[snipped]
1251         WARN_ON(sectors < 0);

The WARN_ON() at line 1257 doesn't make sense because sectors is
unsigned long long type and never to be <0.

Fix it by checking directly checking whether sectors is less than len.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Coly Li <colyli@kernel.org>
---
 block/badblocks.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Coly Li March 9, 2025, 4:12 p.m. UTC | #1 
Hi Jens,

Could you please take a look at it and pick this patch into the for-6.15/block branch? The patch is generated based on the for-6.15/block branch.

Thanks in advance.

Coly Li


> From: Coly Li <colyli@kernel.org>
> 
> In _badblocks_check(), there are lines of code like this,
> 1246         sectors -= len;
> [snipped]
> 1251         WARN_ON(sectors < 0);
> 
> The WARN_ON() at line 1257 doesn't make sense because sectors is
> unsigned long long type and never to be <0.
> 
> Fix it by checking directly checking whether sectors is less than len.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Coly Li <colyli@kernel.org>
> ---
> block/badblocks.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/badblocks.c b/block/badblocks.c
> index 673ef068423a..ece64e76fe8f 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -1242,14 +1242,15 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
> 	len = sectors;
> 
> update_sectors:
> +	/* This situation should never happen */
> +	WARN_ON(sectors < len);
> +
> 	s += len;
> 	sectors -= len;
> 
> 	if (sectors > 0)
> 		goto re_check;
> 
> -	WARN_ON(sectors < 0);
> -
> 	if (unacked_badblocks > 0)
> 		rv = -1;
> 	else if (acked_badblocks > 0)
> -- 
> 2.47.2
>
Yu Kuai March 10, 2025, 2:06 a.m. UTC | #2 
在 2025/03/10 0:05, colyli@kernel.org 写道:
> From: Coly Li <colyli@kernel.org>
> 
> In _badblocks_check(), there are lines of code like this,
> 1246         sectors -= len;
> [snipped]
> 1251         WARN_ON(sectors < 0);
> 
> The WARN_ON() at line 1257 doesn't make sense because sectors is
> unsigned long long type and never to be <0.
> 
> Fix it by checking directly checking whether sectors is less than len.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Coly Li <colyli@kernel.org>
> ---
>   block/badblocks.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 

LGTM
Reviewed-by: Yu Kuai <yukuai3@huawei.com>

> diff --git a/block/badblocks.c b/block/badblocks.c
> index 673ef068423a..ece64e76fe8f 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -1242,14 +1242,15 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>   	len = sectors;
>   
>   update_sectors:
> +	/* This situation should never happen */
> +	WARN_ON(sectors < len);
> +
>   	s += len;
>   	sectors -= len;
>   
>   	if (sectors > 0)
>   		goto re_check;
>   
> -	WARN_ON(sectors < 0);
> -
>   	if (unacked_badblocks > 0)
>   		rv = -1;
>   	else if (acked_badblocks > 0)
>
Jens Axboe March 10, 2025, 1:42 p.m. UTC | #3 
On 3/9/25 10:12 AM, Coly Li wrote:
> Hi Jens,
> 
> Could you please take a look at it and pick this patch into the for-6.15/block branch? The patch is generated based on the for-6.15/block branch.

Just a heads-up - you don't need to send these emails outside of
just sending the patch, I do get the patches. If I didn't, then that'd
be a problem. If you feel patches need extra context, then just do a
cover letter for them.
Coly Li March 10, 2025, 1:44 p.m. UTC | #4 
> 2025年3月10日 21:42，Jens Axboe <axboe@kernel.dk> 写道：
> 
> On 3/9/25 10:12 AM, Coly Li wrote:
>> Hi Jens,
>> 
>> Could you please take a look at it and pick this patch into the for-6.15/block branch? The patch is generated based on the for-6.15/block branch.
> 
> Just a heads-up - you don't need to send these emails outside of
> just sending the patch, I do get the patches. If I didn't, then that'd
> be a problem. If you feel patches need extra context, then just do a
> cover letter for them.

So if you are the receiver of the patch email, then I don’t need to worry that you will treat it as a normal patch for review. Can I take this as a rule?

Thanks.

Coly Li
Jens Axboe March 10, 2025, 1:49 p.m. UTC | #5 
On 3/10/25 7:44 AM, Coly Li wrote:
> 
> 
>> 2025?3?10? 21:42?Jens Axboe <axboe@kernel.dk> ???
>>
>> On 3/9/25 10:12 AM, Coly Li wrote:
>>> Hi Jens,
>>>
>>> Could you please take a look at it and pick this patch into the for-6.15/block branch? The patch is generated based on the for-6.15/block branch.
>>
>> Just a heads-up - you don't need to send these emails outside of
>> just sending the patch, I do get the patches. If I didn't, then that'd
>> be a problem. If you feel patches need extra context, then just do a
>> cover letter for them.
> 
> So if you are the receiver of the patch email, then I don?t need to
> worry that you will treat it as a normal patch for review. Can I take
> this as a rule?

Yes of course - I don't think I've ever seen anyone else send out a
patch with a followup a few minutes later to apply it. Just send out the
patch, and it should get applied. If it doesn't after a week or
whatever, then feel free to send a reminder. But a reminder to apply it
a few min after the original patch is a bit unusual and odd.
Jens Axboe March 10, 2025, 1:50 p.m. UTC | #6 
On Sun, 09 Mar 2025 12:05:56 -0400, colyli@kernel.org wrote:
> In _badblocks_check(), there are lines of code like this,
> 1246         sectors -= len;
> [snipped]
> 1251         WARN_ON(sectors < 0);
> 
> The WARN_ON() at line 1257 doesn't make sense because sectors is
> unsigned long long type and never to be <0.
> 
> [...]

Applied, thanks!

[1/1] badblocks: Fix a nonsense WARN_ON() which checks whether a u64 variable < 0
      commit: 7e76336e14de9a2b67af96012ddd46c5676cf340

Best regards,
Coly Li March 10, 2025, 1:50 p.m. UTC | #7 
> 2025年3月10日 21:49，Jens Axboe <axboe@kernel.dk> 写道：
> 
> On 3/10/25 7:44 AM, Coly Li wrote:
>> 
>> 
>>> 2025?3?10? 21:42?Jens Axboe <axboe@kernel.dk> ???
>>> 
>>> On 3/9/25 10:12 AM, Coly Li wrote:
>>>> Hi Jens,
>>>> 
>>>> Could you please take a look at it and pick this patch into the for-6.15/block branch? The patch is generated based on the for-6.15/block branch.
>>> 
>>> Just a heads-up - you don't need to send these emails outside of
>>> just sending the patch, I do get the patches. If I didn't, then that'd
>>> be a problem. If you feel patches need extra context, then just do a
>>> cover letter for them.
>> 
>> So if you are the receiver of the patch email, then I don?t need to
>> worry that you will treat it as a normal patch for review. Can I take
>> this as a rule?
> 
> Yes of course - I don't think I've ever seen anyone else send out a
> patch with a followup a few minutes later to apply it. Just send out the
> patch, and it should get applied. If it doesn't after a week or
> whatever, then feel free to send a reminder. But a reminder to apply it
> a few min after the original patch is a bit unusual and odd.

Copied. So normally I will not send a following email to explain the extra information. If it is necessary, I will compose a cover letter.

Thanks for the notice.

Coly Li
diff mbox series

Patch

diff --git a/block/badblocks.c b/block/badblocks.c
index 673ef068423a..ece64e76fe8f 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -1242,14 +1242,15 @@  static int _badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
 	len = sectors;
 
 update_sectors:
+	/* This situation should never happen */
+	WARN_ON(sectors < len);
+
 	s += len;
 	sectors -= len;
 
 	if (sectors > 0)
 		goto re_check;
 
-	WARN_ON(sectors < 0);
-
 	if (unacked_badblocks > 0)
 		rv = -1;
 	else if (acked_badblocks > 0)