Some comments inline. Regards
Rüdiger On 19.07.2006 14:18, [EMAIL PROTECTED] wrote: > Author: jfclere > Date: Wed Jul 19 05:18:10 2006 > New Revision: 423444 > --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c > (added) > +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c > Wed Jul 19 05:18:10 2006 > @@ -0,0 +1,145 @@ > +/* Copyright 1999-2006 The Apache Software Foundation or its licensors, as > + * applicable. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. Please use the latest header approved by the board in all files. > +} > +static apr_status_t ap_slotmem_create(ap_slotmem_t **new, const char *name, > apr_size_t item_size, int item_num, apr_pool_t *pool) > +{ > + void *slotmem = NULL; > + ap_slotmem_t *res; > + ap_slotmem_t *next = globallistmem; > + char *fname; > + apr_status_t rv; > + > + fname = ap_server_root_relative(pool, name); > + > + /* first try to attach to existing slotmem */ > + if (next) { > + for (;;) { > + if (strcmp(next->name, fname) == 0) { > + /* we already have it */ > + *new = next; > + return APR_SUCCESS; > + } > + if (next->next) Shouldn't this be (!next->next)? > + break; > + next = next->next; > + } > + } > + > + /* create the memory using the globalpool */ > + res = (ap_slotmem_t *) apr_pcalloc(globalpool, sizeof(ap_slotmem_t)); > + res->base = apr_pcalloc(globalpool, item_size * item_num); > + if (!res->base) > + return APR_ENOSHMAVAIL; > + memset(res->base, 0, item_size * item_num); Is this needed? You did a calloc. > + > + /* For the chained slotmem stuff */ > + res->name = apr_pstrdup(globalpool, fname); > + res->size = item_size; > + res->num = item_num; > + res->next = NULL; > + if (globallistmem==NULL) > + globallistmem = res; > + else > + next->next = res; > + > + *new = res; > + return APR_SUCCESS; > +} > +static apr_status_t ap_slotmem_mem(ap_slotmem_t *score, int id, void**mem) > +{ > + > + void *ptr = score->base + score->size * id; > + > + if (!score) > + return APR_ENOSHMAVAIL; Shouldn't this check happen before assigning a value to ptr? > > Added: > httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c?rev=423444&view=auto > ============================================================================== > --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c > (added) > +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c > Wed Jul 19 05:18:10 2006 > +static apr_status_t ap_slotmem_mem(ap_slotmem_t *score, int id, void**mem) > +{ > + void *ptr; > + if (!score) > + return APR_ENOSHMAVAIL; > + > +#if PROXY_HAS_SCOREBOARD > + if (ap_scoreboard_image) > + ptr = (void *)ap_get_scoreboard_lb(id); > +#else > + return APR_ENOSHMAVAIL; > +#endif > + > + if (!ptr) > + return APR_ENOSHMAVAIL; > + *mem = ptr; > + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, > + "ap_slotmem_mem: score %d", score); Shouldn't this be APLOG_DEBUG? > > Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_sharedmem.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_sharedmem.c?rev=423444&view=auto > ============================================================================== > --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_sharedmem.c > (added) > +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_sharedmem.c > Wed Jul 19 05:18:10 2006 > > +static apr_status_t ap_slotmem_create(ap_slotmem_t **new, const char *name, > apr_size_t item_size, int item_num, apr_pool_t *pool) > +{ > + void *slotmem = NULL; > + ap_slotmem_t *res; > + ap_slotmem_t *next = globallistmem; > + char *fname; > + apr_status_t rv; > + > + fname = ap_server_root_relative(pool, name); > + > + /* first try to attach to existing slotmem */ > + if (next) { > + for (;;) { > + if (strcmp(next->name, fname) == 0) { > + /* we already have it */ > + *new = next; > + return APR_SUCCESS; > + } > + if (next->next) Shouldn't this be (!next->next)? > + break; > + next = next->next; > + } > + } > + > + /* first try to attach to existing shared memory */ > + res = (ap_slotmem_t *) apr_pcalloc(globalpool, sizeof(ap_slotmem_t)); > + rv = apr_shm_attach(&res->shm, fname, globalpool); > + if (rv == APR_SUCCESS) { > + /* check size */ > + if (apr_shm_size_get(res->shm) != item_size * item_num) { > + apr_shm_detach(res->shm); > + res->shm = NULL; > + return APR_EINVAL; > + } > + } else { > + rv = apr_shm_create(&res->shm, item_size * item_num, fname, > globalpool); > + if (rv != APR_SUCCESS) > + return rv; > + memset(apr_shm_baseaddr_get(res->shm), 0, item_size * item_num); > + } > + > + /* For the chained slotmem stuff */ > + res->name = apr_pstrdup(globalpool, fname); > + res->base = apr_shm_baseaddr_get(res->shm); > + res->size = item_size; > + res->num = item_num; > + res->next = NULL; > + if (globallistmem==NULL) > + globallistmem = res; > + else > + next->next = res; > + > + *new = res; > + return APR_SUCCESS; > +} > +static apr_status_t ap_slotmem_mem(ap_slotmem_t *score, int id, void**mem) > +{ > + > + void *ptr = score->base + score->size * id; > + > + if (!score) > + return APR_ENOSHMAVAIL; Shouldn't this check happen before assigning a value to ptr? > + if (id<0 || id>score->num) > + return APR_ENOSHMAVAIL; > + if (apr_shm_size_get(score->shm) != score->size * score->num) > + return APR_ENOSHMAVAIL; Is this check needed here again? > + > + if (!ptr) > + return APR_ENOSHMAVAIL; > + *mem = ptr; > + return APR_SUCCESS; > +} > + > +/* make sure the shared memory is cleaned */ > +static int initialize_cleanup(apr_pool_t *p, apr_pool_t *plog, apr_pool_t > *ptemp, server_rec *s) > +{ > + ap_slotmem_t *next = globallistmem; > + while (next) { next = globallistmem; > + next = next->next; > + } What is the purpose of this loop? > + apr_pool_cleanup_register(p, &globallistmem, cleanup_slotmem, > apr_pool_cleanup_null); > + return OK; > +} > +
