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

["David VomLehn (dvomlehn)" <dvomlehn@cisco.com>]

> -----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.