httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Darroch <chr...@pearsoncmg.com>
Subject Re: svn commit: r726113 - /httpd/httpd/trunk/server/mpm/worker/fdqueue.c
Date Wed, 31 Dec 2008 05:41:14 GMT
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


Mime
View raw message