RE: [PATCH] MaxMemFree directive

[Sander Striker <st...@apache.org>]

> From: Justin Erenkrantz [mailto:jerenkrantz@apache.org]
> Sent: 30 June 2002 09:51

> On Fri, Jun 28, 2002 at 01:28:33PM +0200, Sander Striker wrote:
>> All MPMs that are creating their own allocator* (to get rid of
>> locking overhead), now have the MaxMemFree directive.  MaxMemFree
>> allows you to set the maximum amount of memory (in kB), you want
>> the child allocator to hold on to.  Anything over that threshold
>> is free()d back to the system.  The default stays 'unlimited'.
> 
> +1 in concept.
> 
>> /me just realized we have another magic value to get rid off and instead
>>     do #define APR_ALLOCATOR_MAX_FREE_UNLIMITED  0,  *sigh*
>
> That'd be nice to add when you cleanup the allocator stuff to
> make the types opaque.  =)

*grunt* *grmbl* ;)

I'll do that before 2.0.40, but have something else on my plate right
now.
 
>> +#ifdef AP_MPM_WANT_SET_MAX_MEM_FREE
>> +apr_uint32_t ap_max_mem_free = 0;
>> +
>> +const char *ap_mpm_set_max_mem_free(cmd_parms *cmd, void *dummy,
>> +	                            const char *arg)
>> +{
>> +    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
>> +    if (err != NULL) {
>> +        return err;
>> +    }
>> +    
>> +    ap_max_mem_free = atoi(arg) * 1024;
>> +
>> +    return NULL;
>> +}
> 
> My one caveat would be to enforce that arg is positive.
> atoi() could return a negative, but it'll get casted to an
> unsigned 32-bit quantity.

Ack.  Will fix.

> Or does the config core code reject negative numbers straight out?
> (Way too lazy to check myself.)  I think you'd be safer if you used
> strtol (or better yet strtoul - strtoul is in C89, so that should be
> everywhere).  -- justin

I went to try this and got a compiler error because
 'strtoul_is_not_a_portable_function_use_strtol_instead' doesn't exist.

A quick grep on strtoul gave me:

/opt/src/build/httpd-2.0/CHANGES:8778:  *) Remove strtoul() use from mod_proxy because it isn't available
/opt/src/build/httpd-2.0/include/httpd.h:1684:/** strtoul does not exist on sunos4. */
/opt/src/build/httpd-2.0/include/httpd.h:1685:#ifdef strtoul
/opt/src/build/httpd-2.0/include/httpd.h:1686:#undef strtoul
/opt/src/build/httpd-2.0/include/httpd.h:1688:#define strtoul strtoul_is_not_a_portable_function_use_strtol_instead
/opt/src/build/httpd-2.0/server/core.c:2471:    /* WTF: If strtoul is not portable, then write a replacement.
/opt/src/build/httpd-2.0/server/.#core.c.1.187:2471:    /* WTF: If strtoul is not portable, then write a replacement.
/opt/src/build/httpd-2.0/srclib/pcre/pcretest.c:464:      ((size_offsets = (int)strtoul(argv[op+1], &endptr, 10)), *endptr == 0))

So, to summarize:

 - PCRE uses it
 - At other places we complain because we can't use strtoul because of the define
 - There is no apr_strtoul, which would make sense if it isn't portable
 - C89 is pretty ancient... does sunos still not implement it?

Sander

PS.  Haven't heard any objections, so I've committed MemMaxFree.