httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Querna <c...@force-elite.com>
Subject Re: svn commit: r708462 - in /httpd/httpd/trunk/server/mpm: ./ simple/
Date Tue, 28 Oct 2008 22:41:18 GMT
Ruediger Pluem wrote:
> 
> On 10/28/2008 08:00 AM, pquerna@apache.org wrote:
>> Author: pquerna
>> Date: Tue Oct 28 00:00:15 2008
>> New Revision: 708462
>>
>> URL: http://svn.apache.org/viewvc?rev=708462&view=rev
>> Log:
>> Add a work in progress, a completely new, "Simple MPM".
>>
>> Added:
>>     httpd/httpd/trunk/server/mpm/simple/   (with props)
> 
> Nitpicking: While event MPM is still in experimental simple is not? :-).

I think the whole 'experimental' MPM thing should go away. If its in a 
stable released version, it should be stable, not experimental.... But 
thats a different thread.

Anyways, I would support moving Event out, since we have had the 
clogging input filters / mod_ssl support in there for awhile :-)

> As for name proposals, how about
> 
> Generic MPM
> Universal MPM
> 
> 
>> Added: httpd/httpd/trunk/server/mpm/simple/simple_api.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/simple/simple_api.c?rev=708462&view=auto
>> ==============================================================================
> 
>> +static const char*
>> +set_proccount(cmd_parms *cmd, void *baton, const char *arg)
>> +{
>> +  const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
>> +  if (err != NULL) {
>> +    return err;
>> +  }
>> +  
>> +  simple_core_get()->procmgr.proc_count = atoi(arg);
> 
> Does this work on ANSI compilers?

Doubtful.

>> +  simple_core_t *sc = scon->sc;
>> +  conn_rec *c = scon->c;
>> +  conn_state_t *cs = c->cs;
> 
> This does not work on ANSI C compilers. Declarations need to be at the start of the block.

Then lets not support them.

C90 required that variable decls came before code, but IIRC C99 style 
mid-function declarations should work on pretty much all modern 
platforms, aka anything with GCC or MSVC.

>> +        APR_RING_CHECK_CONSISTENCY(&sc->timer_ring, simple_timer_t, link);
> 
> Do we really need this check here? We have already done this.
> 
>> +  APR_RING_CHECK_CONSISTENCY(&sc->timer_ring, simple_timer_t, link);
> 
> Do we really need this check here? We have already done this.

Ring Consistency Checks are no-ops unless APR_RING_DEBUG is defined, so, 
all of these lines compile out unless you are hunting down a place you 
are corrupting the ring.

I don't view them as important to remove, mostly because I was chasing 
down a bug where I corrupted the timer_ring earlier in the evening.

  > Regards
> 
> RĂ¼diger

Thanks for reviewing,

Paul


Mime
View raw message