httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r425734 - in /httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem: config5.m4 mod_plainmem.c mod_scoreboard.c mod_sharedmem.c sharedmem_util.c slotmem.h
Date Wed, 26 Jul 2006 14:42:55 GMT
On 26.07.2006 15:42, jfclere@apache.org wrote:
> Author: jfclere
> Date: Wed Jul 26 06:42:43 2006
> New Revision: 425734
> 
> URL: http://svn.apache.org/viewvc?rev=425734&view=rev
> Log:
> Add ap_slotmem_attach() to the slotmem_storage_method.
> Cut mod_sharemem.c in 2 so that its features could be
> used outside httpd.

> 
> Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c?rev=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c Wed Jul 26
06:42:43 2006
> @@ -98,6 +98,36 @@
>      *new = res;
>      return APR_SUCCESS;
>  }
> +
> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t
item_size, int item_num, apr_pool_t *pool)

Why using item_size and item_num as call by value parameters? Shouldn't that be
pointers?

> +{
> +    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;
> +                *item_size = next->size;
> +                *item_num = next->num;

See comment above.


> 
> Modified: 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=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c Wed Jul
26 06:42:43 2006
> @@ -84,6 +84,12 @@
>      *new = res;
>      return APR_SUCCESS;
>  }
> +
> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t
*item_size, int *item_num, apr_pool_t *pool)
> +{
> +    return(ap_slotmem_create(new, name, item_size, item_num, pool));

This does not work. You need to provide ap_slotmem_create with values for
item_size and item_num. It does not return them. This can get difficult since
these values get not stored in the scoreboard. There is not globallistmem here.

> 
> Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c?rev=425734&view=auto
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c (added)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c Wed Jul
26 06:42:43 2006

> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t
*item_size, int *item_num, apr_pool_t *pool)
> +{
> +    void *slotmem = NULL;
> +    void *ptr;
> +    ap_slotmem_t *res;
> +    ap_slotmem_t *next = globallistmem;
> +    struct sharedslotdesc desc;
> +    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;
> +                *item_size = next->size;
> +                *item_num = next->num;
> +                return APR_SUCCESS;
> +            }
> +            if (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)
> +        return rv;
> +
> +    /* Read the description of the slotmem */
> +    ptr = apr_shm_baseaddr_get(res->shm);
> +    memcpy(&desc, ptr, sizeof(desc));
> +    ptr = ptr + sizeof(desc);
> +
> +    /* For the chained slotmem stuff */
> +    res->name = apr_pstrdup(globalpool, fname);
> +    res->base = ptr;
> +    res->size = desc.item_size;
> +    res->num = desc.item_num;
> +    res->next = NULL;
> +    if (globallistmem==NULL)
> +        globallistmem = res;
> +    else
> +        next->next = res;
> +
> +    *new = res;
> +    *item_size = desc.item_size;
> +    *item_num = desc.item_num;
> +    return APR_SUCCESS;

Too much copy and paste? We already found it above and what we are searching for
is in *new isn't it? (Maybe we should also set *new to NULL in the beginning to
have a defined return value?)

> 
> Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h?rev=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h Wed Jul 26 06:42:43
2006
> @@ -65,6 +65,17 @@
>  AP_DECLARE(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);
>  
>  /**
> + * attach to an existing slotmem.
> + * This would attach to  shared memory, basically.
> + * @param pointer to store the address of the scoreboard.
> + * @param name is a key used for debugging and in mod_status output or allow another
process to share this space.
> + * @param item_size size of each idem
> + * @param item_num max number of idem.

Guess it should be item above :-)?


Regards

RĂ¼diger



Mime
View raw message