I'm +1 for committing as is and letting people test/patch
from there and then backporting ;)
On Dec 4, 2011, at 10:41 AM, Graham Leggett wrote:
> On 04 Dec 2011, at 5:01 PM, Jim Jagielski wrote:
>
>> As you likely know, the origins of slotmem grew out of the
>> httpd-proxy-scoreboard
>> branch (~5.5 years ago) and I assume jfclere had reasons for his choice
>> for how to setup naming of paths, etc... But that is water under the
>> bridge; if we wish to fix that then instead of throwing insults hither and
>> yon, a really good idea would be to make the suggestion that it be
>> fixed based on history/consistency/etc...
>>
>> I see that Graham has suggested a course of action, and I +1'ed
>> it; if Graham lacks the time, I will find some cycles to address
>> and implement.
>
> I currently have this, which compiles but still needs to be tested. Busy this
> afternoon, so if someone can take a look and see whether this works it would
> help me. store_filename() seems sane, but the "look up existing segment"
> stuff needs to be checked that it matches correctly.
>
> Index: modules/slotmem/mod_slotmem_shm.c
> ===================================================================
> --- modules/slotmem/mod_slotmem_shm.c (revision 1210009)
> +++ modules/slotmem/mod_slotmem_shm.c (working copy)
> @@ -119,29 +119,24 @@
> * Persist the slotmem in a file
> * slotmem name and file name.
> * none : no persistent data
> - * anonymous : $server_root/logs/anonymous.slotmem
> - * :rel_name : $server_root/logs/rel_name.slotmem
> - * abs_name : $abs_name.slotmem
> + * rel_name : $server_root/rel_name
> + * /abs_name : $abs_name
> *
> */
> static const char *store_filename(apr_pool_t *pool, const char *slotmemname)
> {
> const char *storename;
> const char *fname;
> - if (strcasecmp(slotmemname, "none") == 0)
> + if (strcasecmp(slotmemname, "none") == 0) {
> return NULL;
> - else if (strcasecmp(slotmemname, "anonymous") == 0)
> - fname = ap_server_root_relative(pool, "logs/anonymous");
> - else if (slotmemname[0] == ':') {
> - const char *tmpname;
> - tmpname = apr_pstrcat(pool, "logs/", &slotmemname[1], NULL);
> - fname = ap_server_root_relative(pool, tmpname);
> }
> + else if (slotmemname[0] != '/') {
> + fname = ap_server_root_relative(pool, slotmemname);
> + }
> else {
> fname = slotmemname;
> }
> - storename = apr_pstrcat(pool, fname, ".slotmem", NULL);
> - return storename;
> + return fname;
> }
>
> static void store_slotmem(ap_slotmem_instance_t *slotmem)
> @@ -269,14 +264,15 @@
> (item_num * sizeof(char)) + basesize;
> apr_status_t rv;
>
> - if (gpool == NULL)
> + if (gpool == NULL) {
> return APR_ENOSHMAVAIL;
> + }
> if (name) {
> - if (name[0] == ':') {
> - fname = name;
> + if (name[0] != '/') {
> + fname = ap_server_root_relative(pool, name);
> }
> else {
> - fname = ap_server_root_relative(pool, name);
> + fname = name;
> }
>
> /* first try to attach to existing slotmem */
> @@ -295,11 +291,11 @@
> }
> }
> else {
> - fname = "anonymous";
> + fname = "none";
> }
>
> /* first try to attach to existing shared memory */
> - fbased = (name && name[0] != ':');
> + fbased = (name != NULL);
> if (fbased) {
> rv = apr_shm_attach(&shm, fname, gpool);
> }
>
> Regards,
> Graham
> --
>
