linux-mips
[Top] [All Lists]

RE: [PATCH] MIPS: unsigned result is always greater than 0

To: "Roel Kluin" <roel.kluin@gmail.com>, <ralf@linux-mips.org>
Subject: RE: [PATCH] MIPS: unsigned result is always greater than 0
From: "David VomLehn (dvomlehn)" <dvomlehn@cisco.com>
Date: Tue, 6 Jan 2009 16:46:19 -0500
Authentication-results: rtp-dkim-2; header.From=dvomlehn@cisco.com; dkim=pass ( sig from cisco.com/rtpdkim2001 verified; );
Cc: <linux-mips@linux-mips.org>
Dkim-signature: v=1; a=rsa-sha256; q=dns/txt; l=1894; t=1231278387; x=1232142387; c=relaxed/simple; s=rtpdkim2001; h=Content-Type:From:Subject:Content-Transfer-Encoding:MIME-Version; d=cisco.com; i=dvomlehn@cisco.com; z=From:=20=22David=20VomLehn=20(dvomlehn)=22=20<dvomlehn@cis co.com> |Subject:=20RE=3A=20[PATCH]=20MIPS=3A=20unsigned=20result=2 0is=20always=20greater=20than=200 |Sender:=20 |To:=20=22Roel=20Kluin=22=20<roel.kluin@gmail.com>,=20<ralf @linux-mips.org>; bh=jWocxyxDaCUg6B9soHVdslVwborpn2SdrpMosvPKt5g=; b=o1CdR8E+mq0D2iyvmxmn4adI2dGkEh5rR6b5vBKZUAB+3eke1Fsfu3V7Ur JmOMaojtM1NG+J0dARJOWvOdwwCevC5Zr3kKpC4h3HCdN4JK6jCIjmYW4mNT G1mZm080s4;
In-reply-to: <495E2E47.6080605@gmail.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <495E2E47.6080605@gmail.com>
Sender: linux-mips-bounce@linux-mips.org
Thread-index: Acls7EfWnUJEqqivROa5wJWiY2zpvQDWuIJg
Thread-topic: [PATCH] MIPS: unsigned result is always greater than 0
 

> -----Original Message-----
> From: linux-mips-bounce@linux-mips.org 
> [mailto:linux-mips-bounce@linux-mips.org] On Behalf Of Roel Kluin
> Sent: Friday, January 02, 2009 7:10 AM
> To: ralf@linux-mips.org
> Cc: linux-mips@linux-mips.org
> Subject: [PATCH] MIPS: unsigned result is always greater than 0
> 
> unsigned result is always greater than 0
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> I cannot determine whether the same bug occurs as well in assembly.
> Also shouldn't similar checks occur in atomic64_sub_return and in
> atomic64_add_return for negative values of i?
> 
> diff --git a/arch/mips/include/asm/atomic.h 
> b/arch/mips/include/asm/atomic.h
> index 1232be3..3cd07a9 100644
> --- a/arch/mips/include/asm/atomic.h
> +++ b/arch/mips/include/asm/atomic.h
> @@ -296,9 +296,10 @@ static __inline__ int 
> atomic_sub_if_positive(int i, atomic_t * v)
>  
>               raw_local_irq_save(flags);
>               result = v->counter;
> -             result -= i;
> -             if (result >= 0)
> +             if (i <= result) {
> +                     result -= i;
>                       v->counter = result;
> +             }
>               raw_local_irq_restore(flags);
>       }
>  
> @@ -677,9 +678,10 @@ static __inline__ long 
> atomic64_sub_if_positive(long i, atomic64_t * v)
>  
>               raw_local_irq_save(flags);
>               result = v->counter;
> -             result -= i;
> -             if (result >= 0)
> +             if (i >= result) {
> +                     result -= i;
>                       v->counter = result;
> +             }
>               raw_local_irq_restore(flags);
>       }

I agree that the code as it exists is wrong, but, as I see it, the
problem is that the type of result should be changed from unsigned long
to int. This fixes the comparison so it works correctly. In addition,
such a change means that result would be the same type as the counter
element of atomic_t, avoiding possible surprises should longs be larger
than ints.

<Prev in Thread] Current Thread [Next in Thread>