Messages in this thread | | | From | Kees Cook <> | Date | Thu, 1 Jun 2017 07:43:37 -0700 | Subject | Re: [PATCH v5 1/3] refcount: Create unchecked atomic_t implementation |
| |
On Wed, May 31, 2017 at 4:09 AM, Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, May 31, 2017 at 10:45:09AM +0000, Reshetova, Elena wrote: > >> > +static inline __must_check bool refcount_add_not_zero(unsigned int i, >> > + >> > refcount_t *r) >> > +{ >> > + return atomic_add_return(i, &r->refs) != 0; >> > +} >> >> Maybe atomic_add_unless(&r->refs, i, 0) in order to be consistent with the below inc_not_zero implementation? > > Yes, atomic_add_return() is strictly incorrect here since the add is > unconditional. > >> > +static inline __must_check bool refcount_sub_and_test(unsigned int i, >> > + >> > refcount_t *r) >> > +{ >> > + return atomic_sub_return(i, &r->refs) == 0; >> > +} >> >> Any reason for not using atomic_sub_and_test() here? > >> > +static inline __must_check bool refcount_dec_and_test(refcount_t *r) >> > +{ >> > + return atomic_dec_return(&r->refs) == 0; >> > +} >> >> Same here: atomic_dec_and_test()? > > Both those are better because they return condition codes generated from > the operand itself.
Ah yes, thanks to both of you for the corrections. I'll send a new version...
-Kees
-- Kees Cook Pixel Security
| |