Re: mod_log_forensic?

[André Malo <nd...@perlig.de>]

* Jeff Trawick <tr...@attglobal.net> wrote:

> André Malo wrote:
> > * Jeff Trawick <tr...@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.

Additionally there's no Win32 section in apr/atomic in 0.9.
-> there needs to be done some apr work before porting mod_log_forensic.

[cc dev@apr]

nd

Re: mod_log_forensic?

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

At 06:49 AM 3/29/2004, Jeff Trawick wrote:
>Ben Laurie wrote:
>>Jeff Trawick wrote:
>
>>>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?
>
>a) agreed

Let me remind that although most platform compilers and linkers 
are agnostic to the retval, some may optimize in such a way that
adding or eliminating a retval from an exported function declatation
could break binary compatibility.  I don't care either way - just being
pedantic in the declaration. 

If we didn't toss out apr 0.9.5-final then this is a good place to throw it.
Add an explicit test in th 2.0.x flavor of mod_unique_id to scream that
apr 0.9.5 released or later 0.9 is required.

Bill  

Re: mod_log_forensic?

[Jeff Trawick <tr...@attglobal.net>]

Ben Laurie wrote:
> Jeff Trawick wrote:

> We could make the 2.0.x version require mod_unique_id.

that seems very reasonable

>> 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?

a) agreed

b) if some miracle occurred and a tested patch for all platforms to do the 
right thing became available, then agreed; hard to solve such an issue in 
increments; past experience here shows it is hard to solve any atomic issue 
without multiple increments

Re: mod_log_forensic?

[Ben Laurie <be...@algroup.co.uk>]

Jeff Trawick wrote:

> André Malo wrote:
> 
>> * Jeff Trawick <tr...@attglobal.net> wrote:
>>
>>
>>> André Malo wrote:
>>>
>>>> * Jeff Trawick <tr...@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

Re: mod_log_forensic?

[Jeff Trawick <tr...@attglobal.net>]

André Malo wrote:
> * Jeff Trawick <tr...@attglobal.net> wrote:
> 
> 
>>André Malo wrote:
>>
>>>* Jeff Trawick <tr...@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

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