Ruediger Pluem wrote:
Some comments inline.
Regards
Many thanks
Cheers
Jean-Frederic
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.
Done.
+}
+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)?
Yep.
+ 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.
Argh too fast in reusing the shared memory code.
+
+ /* 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?
Well after all the checks.
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?
I have removed the ap_log_error().
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)?
Yep.
+ 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_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?
Yes.
+ 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?
No
+
+ 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?
Oops that was debugging code.
+ apr_pool_cleanup_register(p, &globallistmem, cleanup_slotmem,
apr_pool_cleanup_null);
+ return OK;
+}
+
