> -----Original Message----- > From: linux-mips-bounce@xxxxxxxxxxxxxx > [mailto:linux-mips-bounce@xxxxxxxxxxxxxx] On Behalf Of Roel Kluin > Sent: Friday, January 02, 2009 7:10 AM > To: ralf@xxxxxxxxxxxxxx > Cc: linux-mips@xxxxxxxxxxxxxx > Subject: [PATCH] MIPS: unsigned result is always greater than 0 > > unsigned result is always greater than 0 > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > --- > 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.
