replace __sync_fetch_and_OP with __sync_OP_and_fetch

[ikruglov]

Hi,

I know that this patch is a bit irritating and it will not make any difference at all. But I wanted to contribute to such a cool software and found low hanging fruit :-)

Note that I changed op in ATOMIC_READ to OR.

Also, it's quite possible that I'm stupid and you wrote this for very good and very smart reasons :-) If so, just disregard me (but explain, please :-))

P.S. return value of REFCNT_ATOMIC_INCREMENT/DECREMENT is never checked

[xant]

yes.. usually it's ignored because not relevant... but , if relevant, I think it's better to return the previous (old) value and not the new one, hence the choice of using __sync_fetch_and_OP ( it was a thoughtful choice and not just a random one ;) )

Is there any reason why you wanted to replace it?

[ikruglov]

Okay, make sense about increment/decrement. But not for ATOMIC_READ: old and new values are equal there.

Anyway, feel free to reject the PR.

12 июня 2014 г., в 10:02, Andrea Guzzo notifications@github.com написал(а):

__sync_fetch_and_OP

[xant]

true, and on the ATOMIC_READ you probably prefer to first fetch the old value and then add 0 (which is irrelevant since you are anyway not going to modify the value and it's there more for safety in case of accessing some integer which doesn't fit in a register and to shut up the thread sanitiser).

Actually I don't see any reason to change the call in ATOMIC_READ() .... but I'm open to hear your motivations for changing it.

Anyway, consider those macros are there for refcnt.c and they are not really meant to be exposed ... I think only queue.c (which uses refcnt.c) is making use of them .... but I was thinking about removing them from the header and just redefine the 2 or 3 macros used in queue.c directly there.

You can find a more generic header to wrap the atomic builtins in https://github.com/xant/libshardcache/blob/master/src/atomic.h

but anyway ... I didn't think such macros as something which should be exposed by the library ... they are more stuff used internally to shorten names and make certain repetitive code patterns more readable and less intrusive in the outer logic.

[ikruglov]

moved all definitions of ATOMIC_* to atomic_defs.h

please note how I patched ATOMIC_SET to work with pointers:

#define ATOMIC_SET(__v, __n) {\
    int __o = ATOMIC_READ(__v);\
    if (__builtin_expect(__o != (__n), 1)) {\

was replaced by

#define ATOMIC_SET(__v, __n) {\
    if (__builtin_expect(ATOMIC_READ(__v) != (__n), 1)) {\