apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <...@algroup.co.uk>
Subject Re: mod_log_forensic?
Date Mon, 29 Mar 2004 12:32:33 GMT
Jeff Trawick wrote:

> André Malo wrote:
> 
>> * Jeff Trawick <trawick@attglobal.net> wrote:
>>
>>
>>> André Malo wrote:
>>>
>>>> * Jeff Trawick <trawick@attglobal.net> wrote:
>>>>
>>>>
>>>>
>>>>> somehow I doubt there will be any problems at all getting it 
>>>>> approved, but
>>>>> nobody acted as a champion thus far and asked for approval themselves
>>>>
>>>>
>>>>
>>>> In fact, I've thought it was by intention, because of the APR 1.0 
>>>> atomic
>>>> calls ;-)
>>>
>>>
>>> shouldn't be any serious problems to work with APR 0.9...
>>>
>>> s/apr_atomic_inc32/apr_atomic_inc/
>>> s/apr_uint32_t next_id/apr_atomic_t next_id/
>>>
>>> YMMVslightly
>>
>>
>>
>> Yes, slightly ;-)
>> apr_atomic_inc32 and apr_atomic_inc are not compatible,
>> most notable the interface of the latter is not thread safe.
> 
> 
> IOW apr_atomic_inc() doesn't return a value so the caller knows what it 
> became :(  I forgot that got resolved with 1.0.
> 
> apr_atomic_cas() could theoretically be used.
> 
>> Additionally there's no Win32 section in apr/atomic in 0.9.
> 
> 
> The Win32 implementation of atomics for 0.9 is apr_atomic.h.
> 
>  > -> there needs to be done some apr work before porting mod_log_forensic.
> 
> What do you want to happen?  Add a new increment API to 0.9 that returns 
> a value?
> 
> There would seem to be several solutions, some with APR dependencies and 
> some without.
> 
> solution 1: do what mod_unique_id does for generation of the id (no 
> mutex, no atomic); that seems to be optimal solution

We could make the 2.0.x version require mod_unique_id.

> solution 2: get a mutex explicitly; a bit of mess in the code 
> (APR_HAS_THREADS), but not so sucky performance-wise compared with the 
> apr "atomics" since for many users the apr atomics will be using a mutex 
> under the covers anyway
> 
> solution 3: see if apr_atomic_cas() is implemented well enough to use
> 
> solution 4: add some suitable API to APR 0.9 and implement on all platforms

Surely not returning the value from the inc is broken anyway? And a 
harmless change even if you don't consider it broken?

-- 
http://www.apache-ssl.org/ben.html       http://www.thebunker.net/

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Mime
View raw message