2014-07-18 15:35:00

by Andrey Utkin

Subject: [PATCH 4/5] kernel/debug/kdb/kdb_bp.c: drop negativity check on unsigned value

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=80591
Reported-by: David Binderman <[email protected]>
Signed-off-by: Andrey Utkin <[email protected]>
---
kernel/debug/kdb/kdb_bp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
index 70a5046..f27dfb8 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -39,7 +39,7 @@ static char *kdb_rwtypes[] = {

static char *kdb_bptype(kdb_bp_t *bp)
{
- if (bp->bp_type < 0 || bp->bp_type > 4)
+ if (bp->bp_type > 4)
return "";

return kdb_rwtypes[bp->bp_type];
--
1.8.5.5

2014-07-18 17:18:29

by walter harms

Subject: Re: [PATCH 4/5] kernel/debug/kdb/kdb_bp.c: drop negativity check on unsigned value

Am 18.07.2014 17:34, schrieb Andrey Utkin:
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=80591
> Reported-by: David Binderman <[email protected]>
> Signed-off-by: Andrey Utkin <[email protected]>
> ---
> kernel/debug/kdb/kdb_bp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
> index 70a5046..f27dfb8 100644
> --- a/kernel/debug/kdb/kdb_bp.c
> +++ b/kernel/debug/kdb/kdb_bp.c
> @@ -39,7 +39,7 @@ static char *kdb_rwtypes[] = {
>
> static char *kdb_bptype(kdb_bp_t *bp)
> {
> - if (bp->bp_type < 0 || bp->bp_type > 4)
> + if (bp->bp_type > 4)
> return "";
>
> return kdb_rwtypes[bp->bp_type];

i would expect a ckeck for the upper limit with ARRAY_SIZE(bp->bp_type) here.

re,
wh

2014-07-18 17:41:04

by Andrey Utkin

Subject: Re: [PATCH 4/5] kernel/debug/kdb/kdb_bp.c: drop negativity check on unsigned value

2014-07-18 20:18 GMT+03:00 walter harms <[email protected]>:
> i would expect a ckeck for the upper limit with ARRAY_SIZE

Thanks, resubmitted.

--
Andrey Utkin

2014-07-22 07:47:41

by Dan Carpenter

Subject: Re: [PATCH 4/5] kernel/debug/kdb/kdb_bp.c: drop negativity check on unsigned value

On Fri, Jul 18, 2014 at 06:34:52PM +0300, Andrey Utkin wrote:
> static char *kdb_bptype(kdb_bp_t *bp)
> {
> - if (bp->bp_type < 0 || bp->bp_type > 4)
> + if (bp->bp_type > 4)
> return "";

With Smatch, I ignore negative checks in this format. It's obvious what
the intent is and they are harmless. Patching them requires a little
review to make sure that someone isn't introducing a bug and can't be
done directly in the email client.

On the other hand, in Smatch I do complain about checks like:

if (bp->bp_type > 4 || bp->bp_type < 0)

Because only backwards thinking people write checks like that.

regards,
dan carpenter