| Summary: | apr_atomic_casptr has wrong volatile qualifier | ||
|---|---|---|---|
| Product: | APR | Reporter: | philip |
| Component: | APR | Assignee: | Apache Portable Runtime bugs mailinglist <bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | danielsh |
| Priority: | P2 | Keywords: | FixedInTrunk |
| Version: | HEAD | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | apr_atomic_casptr volatile fix | ||
Yes, this is noted in STATUS as one of the API changes needed for 2.0 - I think it constitutes an API change so we can't really do it in 1.x. I'm late to the party, but why not add an apr_atomic_casptr2() with a corrected signature? Fixed in trunk already, but +1 for apr_atomic_casptr2() in 1.8.x (it seems that 1.7.x lost this opportunity already per our API/ABI compat policy..). Two new functions with the correct API have been added to 1.8.x (r1902268): APR_DECLARE(void *) apr_atomic_casptr2(void *volatile *mem, void *with, const void *cmp); APR_DECLARE(void *) apr_atomic_xchgptr2(void *volatile *mem, void *with); The apr_atomic_casptr() and apr_atomic_xchgptr() were changed (in place) in r1763666, so 2.x (and late)r users can use that ones already. |
Created attachment 26622 [details] apr_atomic_casptr volatile fix The prototype for apr_atomic_casptr is APR_DECLARE(void*) apr_atomic_casptr(volatile void **mem, void *with, const void *cmp); The mem parameter has the wrong volatile qualifier, it is declared as "a non-volatile pointer to a non-volatile pointer to volatile data" but CAS operates on the value of the pointer, not on the value of the dereferenced pointer, so it is the value of the pointer that should be volatile. The correct prototype is APR_DECLARE(void*) apr_atomic_casptr(void * volatile *mem, void *with, const void *cmp); This matches apr_atomic_cas32 where CAS applies to the int32 value and the corresponding parameter is a "pointer to a volatile int32". The function apr_atomic_xchgptr has a similar problem. The attached patch has been tested on 64-bit Linux.