Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
sys/sys/_atomic_subword.h | ||
---|---|---|
209 | Would the normal pattern not be: #ifndef atomic_set_16 static __inline void atomic_set_short(volatile uint16_t *p, uint16_t bit) { ... } #endif and then let each arch do #define atomic_set_short atomic_set_16? That is, (a) invert the direction for the #define'ing (b) split the #define out to per-arch headers (c) guard each definition. This both deviates from the current style but also won't honour architectures that already provide this width of atomic. |
The implementation seems ok to me.
sys/sys/_atomic_subword.h | ||
---|---|---|
209 | Indeed, why not follow the existing pattern? |
Both args' types of atomic_set/clear_16 from _atomic_subword.h were changed to uint16_t.
sys/arm/include/atomic.h | ||
---|---|---|
1105–1106 | Comment is now stale (I *think* prior to this change it was accurate?) | |
sys/powerpc/include/atomic.h | ||
1135 | Should this and the two new #defines not just be hoisted? AFAIUI the only reason the header is made conditional today is because it's unnecessary, not because there's a problem with making it unconditional? |
sys/arm/include/atomic.h | ||
---|---|---|
1105–1106 | Why? I did not changed anything WRT atomic_load. Do you mean that atomic_clear/set_16 should be included in the list? [I would rather remove this comment at all then] | |
sys/powerpc/include/atomic.h | ||
1135 | Sorry I do not understand your comment. The subword use is needed for !206_ATOMIC case, but is too much for 206 so I added the functions definitions directly. |
sys/arm/include/atomic.h | ||
---|---|---|
1105–1106 | Yes, I mean that the list is now misleading since the header is no longer included just for the functions enumerated here. Deleting it is also fine with me, neither of the other arches here include such a list. | |
sys/powerpc/include/atomic.h | ||
1135 | I mean that, since the #else is a copy of the subword code, ideally we'd just include the subword header, in such a way that it doesn't pick up any of the other subword bits, which I suspect would already be the case. But if you would rather not do that still I won't object further. |
sys/powerpc/include/atomic.h | ||
---|---|---|
1135 | I do not want to spend that much efforts on ppc, esp. because I cannot test (but not only). |
sys/powerpc/include/atomic.h | ||
---|---|---|
1135 | Ok fair enough |