Hi --
I wrote:
What we had before was:
if (apr_atomic_casptr((volatile void**)&(queue_info->recycled_pools),
new_recycle, next) == next) {
but also:
if (apr_atomic_cas32(&(queue_info->idlers), prev_idlers + 1,
prev_idlers) == prev_idlers) {
I see no reason why a pointer to an integer doesn't need a
volatile cast, while a pointer to a pointer does. I'm also not
aware of any problems reported caused by the lack of a cast on these
other apr_atomic_*() calls.
Consider too that all the apr_atomic_*() functions explicitly cast
their arguments as pointers to volatile data, so the compiler should
perform no optimization on data accesses within the function
(which is presumably where it counts). Putting a volatile cast on
the argument in the call could only mean something, it seems to me,
if the APR functions didn't explicitly do this already.
Further, the (volatile void**) casts for the recycled_pools pointer
were only added in the first place, according to the commit log, to
avoid compiler warnings (see r101236) -- which they no longer do for
gcc anyway, even if they ever did for some platform and compiler in
the past.
That all said, I did some additional reading on void** casts
and the meaning of the type-punning warning gcc generates, and
it would seem that the underlying reason for the warning here has to
do exclusively with casting a pointer to data (struct recycled_pool*)
to a void*. See http://c-faq.com/ptrs/genericpp.html
for what seems like a pretty decent explanation.
Based on that, it would appear to me that the issue with casting
the arguments passed to apr_atomic_casptr() is that should httpd
be compiled on some unusual platform where a void* is, say, larger
than a struct recycled_pool*, things might go awry.
Now, there are a number of APR functions which take void**
arguments, and I haven't thought about any of these except for the
two apr_atomic_*() ones.
Does anyone know if we should be concerned about any modern
platforms where void* is a different size than some other pointers?
(E.g., pointers to data and pointers to functions might be different
sizes, and void* the larger of the two?) In such a case, we might
need to refactor some code, including these apr_atomic_casptr()
calls. If not, then casting to void* to silence the warnings ought
to be tolerable, I think.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
